Use-After-Free problem

[sahandevs]

Recently I've compiled nginx with GCC sanitiser and I saw following error from this module:

READ of size 4494 at 0x62d0019f06ab thread T0
    #0 0x7f7a8432c982 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:806
    #1 0x55c359711e89 in ngx_ssl_send_chain src/event/ngx_event_openssl.c:2767
    #2 0x55c3597c7a94 in ngx_http_v2_send_output_queue src/http/v2/ngx_http_v2.c:557
    #3 0x55c3597cd044 in ngx_http_v2_write_handler src/http/v2/ngx_http_v2.c:502
    #4 0x55c35970515f in ngx_epoll_process_events src/event/modules/ngx_epoll_module.c:930
    #5 0x55c3596e7c48 in ngx_process_events_and_timers src/event/ngx_event.c:275
    #6 0x55c3596ffbe0 in ngx_worker_process_cycle src/os/unix/ngx_process_cycle.c:823
    #7 0x55c3596fbb45 in ngx_spawn_process src/os/unix/ngx_process.c:199
    #8 0x55c3596fd56c in ngx_start_worker_processes src/os/unix/ngx_process_cycle.c:354
    #9 0x55c359700ec6 in ngx_master_process_cycle src/os/unix/ngx_process_cycle.c:131
    #10 0x55c35968d1d1 in main src/core/nginx.c:383
    #11 0x7f7a83c3cd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
    #12 0x55c359689119 in _start (/app/nginx+0x52e119)

0x62d0019f06ab is located 683 bytes inside of 32439-byte region [0x62d0019f0400,0x62d0019f82b7)
freed by thread T0 here:
    #0 0x7f7a8439cb6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
    #1 0x55c35969125b in ngx_pfree src/core/ngx_palloc.c:286
    #2 0x55c35980790c in ngx_http_brotli_filter_free /app/nginx/ngx_brotli-master/filter/ngx_http_brotli_filter_module.c:617
    #3 0x7f7a8426d9e9 in BrotliEncoderDestroyInstance (/usr/local/lib/libbrotlienc.so.1+0x339e9)

previously allocated by thread T0 here:
    #0 0x7f7a8439ce8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x55c3596f2ffe in ngx_alloc src/os/unix/ngx_alloc.c:22
    #2 0x55c359690866 in ngx_palloc_large src/core/ngx_palloc.c:220
    #3 0x55c359690eeb in ngx_palloc src/core/ngx_palloc.c:131
    #4 0x55c359807917 in ngx_http_brotli_filter_alloc /app/nginx/ngx_brotli-master/filter/ngx_http_brotli_filter_module.c:600
    #5 0x7f7a842709bc  (/usr/local/lib/libbrotlienc.so.1+0x369bc)

SUMMARY: AddressSanitizer: heap-use-after-free ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:806 in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x0c5a80336080: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a80336090: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a803360a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a803360b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a803360c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c5a803360d0: fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd
  0x0c5a803360e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a803360f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a80336100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a80336110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c5a80336120: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd

After looking at the code I suspect the module may destroy the encoder early. for example here:

      out = (u_char*)BrotliEncoderTakeOutput(ctx->encoder, &available_output);
      if (out == NULL || available_output == 0) {
        ngx_http_brotli_filter_close(ctx);
        return NGX_ERROR;
      }

if there is no more output this destroys the encoder. Destroying the encoder causes all allocations to get freed but here it didn't check anywhere to make sure all data were actually sent to the client.

NOTE: I don't know how to reproduce the issue and I only saw this in the production environment.

Am I missing something?

[dimatakoy]

The funny thing is that absolutely no one from Google has even responded to this after half a year.

[eustas]

Ooops. Somehow missed the original message. Of course that requires fixing. Moreover - adding to oss-fuzz, so it would do continuous fuzzing. Will take care of this soon.

[NicolasCARPi]

@eustas Is there someone at Google, among the 190.000 employees, that can give some love to this project?

#144 / #120 would also be great.

Cheers,
~Nico

[eustas]

I'm sorry, it seems that there is only me. Though there are good news - I've been able to allocate a slice of me for giving love to brotli / ngx_brotli / brunsli this year... Currently I'm working on brotli. ngx_brotli is next in my list. Thank you for your interest and patience.

[NicolasCARPi]

@eustas Ok, I'm glad you can continue to work on it. I'm a bit surprised that:

• brotli support is not integrated directly in nginx as an official module
• Google, always working toward faster and leaner web, isn't more invested than that in that project which seems pretty important to me, but maybe I give brotli too much credit, I don't know...

Anyway, I think tagging releases would be a quick win, so we can stop requesting a particular commit like animals. Let me know if I can help by testing branches or PR!

Best,
~Nico