h2o/picotls

Occasional NULL reference when getting ClientHello

tiferrei opened this issue · 2 comments

tiferrei commented

Summary

When attempting to make a ClientHello message with ptls_handle_message and a null input, sometimes this results in a segmentation fault caused by ptls_buffer__do_pushv trying to memcpy from a null src.

Backtrace

#0  0x00007fe106167ce1 in memcpy () from /lib/ld-musl-x86_64.so.1
#1  0x00000000006fe00b in ptls_buffer__do_pushv (len=7604480, src=0x0, buf=0xc00000e6c0)
    at /go/src/github.com/mpiraux/pigotls/picotls/lib/picotls.c:537
#2  ptls_buffer__do_pushv (buf=0xc00000e6c0, src=0x0, len=7604480) at /go/src/github.com/mpiraux/pigotls/picotls/lib/picotls.c:529
#3  0x0000000000709e36 in send_client_hello (tls=tls@entry=0x23672a0, emitter=emitter@entry=0x7fe0df181960,
    properties=properties@entry=0xc0001b2180, cookie=cookie@entry=0x0) at /go/src/github.com/mpiraux/pigotls/picotls/lib/picotls.c:1964
#4  0x000000000070b602 in ptls_client_handle_message (tls=0x23672a0, sendbuf=<optimized out>, epoch_offsets=<optimized out>, in_epoch=0,
    input=<optimized out>, inlen=<optimized out>, properties=0xc0001b2180) at /go/src/github.com/mpiraux/pigotls/picotls/lib/picotls.c:5284
#5  0x00000000006eb102 in _cgo_2c1b90da13a4_Cfunc_ptls_handle_message (v=0xc000285380) at cgo-gcc-prolog:342
#6  0x0000000000467ce0 in runtime.asmcgocall () at /usr/local/go/src/runtime/asm_amd64.s:655
#7  0x0000000000462f86 in runtime.mallocgc.func1 () at /usr/local/go/src/runtime/malloc.go:1047
#8  0x000000c000430000 in ?? ()
#9  0x000000000043e290 in ?? () at <autogenerated>:1
#10 0x0000000000000000 in ?? ()

Environment

Background

  • I'm making use of mpiraux/pigotls as a Go wrapper for picotls, however I don't believe this to be the cause of the issue.
  • I see this issue when sometimes attempting to repeatedly get ClientHello messages. It does not always occur, however with my use case I need to quite frequently generate these messages at any point of the connection. It might happen immediately at the first attempt, or at the 100th.

I can trigger the issue on demand via my use case, however this use case is quite a complex piece of software with different moving parts, so it is likely not helpful with debugging. As such I tried to narrow down the information and variables with a core dump of the issue that can be used with GDB.
I built a Docker image to interact with the core dump, as each core dump will only work for the specific binary it was taken from. To use it simply do:

docker run -ti tiferrei/quic-adapter:coredump

I hope this is enough to help debug the issue, however please do let me know if I can provide you with anything else that might be useful.

Thank you,
Tiago

kazuho commented

I would presume that this is an error in the caller failing to initialize some of the arguments being passed to ptls_handshake.

Looking at the three lines below, ptls_buffer__do_pushv is supplied a NULL pointer (as you correctly point out), which is accompanied by a length of 7604480 bytes, from the giant code block that generates ClientHello.

#2  ptls_buffer__do_pushv (buf=0xc00000e6c0, src=0x0, len=7604480) at /go/src/github.com/mpiraux/pigotls/picotls/lib/picotls.c:529
#3  0x0000000000709e36 in send_client_hello (tls=tls@entry=0x23672a0, emitter=emitter@entry=0x7fe0df181960,
    properties=properties@entry=0xc0001b2180, cookie=cookie@entry=0x0) at /go/src/github.com/mpiraux/pigotls/picotls/lib/picotls.c:1964

I think this might allow you to track down the cause, as in most places, we check if the supplied pointer is NULL. To give one example, in lines 2060 - 2064 we check if cookie->base is NULL before caling ptls_buffer_pushv.

There are only two locations where we call ptls_buffer_pushv without checking the pointer, with length supplied from outside of picotls. Thery are:

  • Elements of ALPN:

    picotls/lib/picotls.c

    Lines 2019 to 2031 in c5e7c8a

    if (properties != NULL && properties->client.negotiated_protocols.count != 0) {
    buffer_push_extension(sendbuf, PTLS_EXTENSION_TYPE_ALPN, {
    ptls_buffer_push_block(sendbuf, 2, {
    size_t i;
    for (i = 0; i != properties->client.negotiated_protocols.count; ++i) {
    ptls_buffer_push_block(sendbuf, 1, {
    ptls_iovec_t p = properties->client.negotiated_protocols.list[i];
    ptls_buffer_pushv(sendbuf, p.base, p.len);
    });
    }
    });
    });
    }
  • Resumption ticket:

    picotls/lib/picotls.c

    Lines 2082 to 2083 in c5e7c8a

    ptls_buffer_push_block(sendbuf, 2,
    { ptls_buffer_pushv(sendbuf, resumption_ticket.base, resumption_ticket.len); });

I think you might want to make sure that these values are correctly provided.

tiferrei commented

Thanks @kazuho, I can now confirm this is a memory issue with the Go wrapper. Closing as resolved.

  • 👍1