openziti / tlsuv

TLS and HTTP(s) client library for libuv
https://docs.openziti.io/tlsuv/
MIT License
45 stars 7 forks source link

Can't free handle in tlsuv_websocket_close callback #177

Closed snej closed 1 year ago

snej commented 1 year ago

In general, if a client allocates a libuv handle on the heap, it can safely free that memory in the callback to uv_close. The libuv docs say this explicitly:

void uv_close(uv_handle_t *handle, uv_close_cb close_cb) Request handle to be closed. close_cb will be called asynchronously after this call. This MUST be called on each handle before memory is released. Moreover, the memory can only be released in close_cb or after it has returned.

However, tlsuv_websocket_close doesn't work this way: it continues to dereference the handle after the close callback, so if the callback frees/invalidates the memory, Bad Stuff will happen.

The following code that frees the tlsuv_websocket_t in the callback will trigger an Address Sanitizer fatal error, or just likely corrupt the heap if AS isn't enabled:

tlsuv_websocket_close(_handle, [](uv_handle_t* h) noexcept {
    free(h);
});

Here's what happens:

ERROR: AddressSanitizer: heap-use-after-free on address 0x000103fdd0a8 at pc 0x000100a511d4 bp 0x00016fdfc7b0 sp 0x00016fdfc7a8
READ of size 8 at 0x000103fdd0a8 thread T0
    #0 0x100a511d0 in on_ws_close websocket.c:435
    #1 0x100a50d20 in ws_close_cb websocket.c:457
    #2 0x100a20d34 in uv_link_close_join uv_link_t.c:126
    #3 0x100a17a14 in uv_link_source_close_cb uv_link_source_t.c:159
    #4 0x1007f89a0 in uv__finish_close core.c:350
    #5 0x1007f5384 in uv__run_closing_handles core.c:364
    #6 0x1007f51f0 in uv_run core.c:462
...

The problem is in on_ws_close, where it calls the close callback but then keeps using ws:

    if (!ws->closed && ws->close_cb) {
        ws->close_cb((uv_handle_t *) ws);
    }

    if (ws->req) {        // <-- Thread 1: Use of deallocated memory

I haven't tried it yet, but I think moving the ws->close_cb call to the end of the function would fix the problem.