Skip to content

Commit

Permalink
Fix stack use-after-free in QUIC
Browse files Browse the repository at this point in the history
When running test_quicapi on master on a Fedora 38 with santizier, a stack
use-after-free is reported:

```
75-test_quicapi.t ..
=================================================================
==28379==ERROR: AddressSanitizer: stack-use-after-return on address 0x03ffa22a2961 at pc 0x03ffa507384a bp 0x03fffb576d68 sp 0x03fffb576550
READ of size 8 at 0x03ffa22a2961 thread T0
    #0 0x3ffa5073849 in memcpy (/usr/lib64/libasan.so.8+0x73849) (BuildId: ce24d4ce2e06892c2e9105155979b957089a182c)
    #1 0x118b883 in tls_handle_alpn ssl/statem/statem_srvr.c:2221
    #2 0x111569d in tls_parse_all_extensions ssl/statem/extensions.c:813
    #3 0x118e2bf in tls_early_post_process_client_hello ssl/statem/statem_srvr.c:1957
    #4 0x118e2bf in tls_post_process_client_hello ssl/statem/statem_srvr.c:2290
    #5 0x113d797 in read_state_machine ssl/statem/statem.c:712
    #6 0x113d797 in state_machine ssl/statem/statem.c:478
    #7 0x10729f3 in SSL_do_handshake ssl/ssl_lib.c:4669
    #8 0x11cec2d in ossl_quic_tls_tick ssl/quic/quic_tls.c:717
    #9 0x11afb03 in ch_tick ssl/quic/quic_channel.c:1296
    #10 0x10cd1a9 in ossl_quic_reactor_tick ssl/quic/quic_reactor.c:79
    #11 0x10d948b in ossl_quic_tserver_tick ssl/quic/quic_tserver.c:160
    #12 0x1021ead in qtest_create_quic_connection test/helpers/quictestlib.c:273
    #13 0x102b81d in test_quic_write_read test/quicapitest.c:54
    #14 0x12035a9 in run_tests test/testutil/driver.c:370
    #15 0x1013203 in main test/testutil/main.c:30
    #16 0x3ffa463262b in __libc_start_call_main (/usr/lib64/libc.so.6+0x3262b) (BuildId: 6bd4a775904d85009582d6887da4767128897d0e)
    openssl#17 0x3ffa463272d in __libc_start_main_impl (/usr/lib64/libc.so.6+0x3272d) (BuildId: 6bd4a775904d85009582d6887da4767128897d0e)
    openssl#18 0x101efb9  (/root/openssl/test/quicapitest+0x101efb9) (BuildId: 075e387adf6d0032320aaa18061f13e9565ab481)
Address 0x03ffa22a2961 is located in stack of thread T0 at offset 33 in frame
    #0 0x10d868f in alpn_select_cb ssl/quic/quic_tserver.c:49
  This frame has 1 object(s):
    [32, 41) 'alpn' (line 50) <== Memory access at offset 33 is inside this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-return (/usr/lib64/libasan.so.8+0x73849) (BuildId: ce24d4ce2e06892c2e9105155979b957089a182c) in memcpy
Shadow bytes around the buggy address:
  0x03ffa22a2680: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2700: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2780: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2800: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2880: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
=>0x03ffa22a2900: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5[f5]f5 f5 f5
  0x03ffa22a2980: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2a00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2a80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2b00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x03ffa22a2b80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==28379==ABORTING
../../util/wrap.pl ../../test/quicapitest default ../../test/default.cnf ../../test/certs => 1
not ok 1 - running quicapitest
```

Fix this be making the protocols to select static constants and thereby moving
them out of the stack frame of the callback function.

Signed-off-by: Juergen Christ <[email protected]>

Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#20904)
  • Loading branch information
juergenchrist authored and mattcaswell committed May 9, 2023
1 parent 3868807 commit ca9ef8e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion ssl/quic/quic_tserver.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ static int alpn_select_cb(SSL *ssl, const unsigned char **out,
unsigned char *outlen, const unsigned char *in,
unsigned int inlen, void *arg)
{
unsigned char alpn[] = { 8, 'o', 's', 's', 'l', 't', 'e', 's', 't' };
static const unsigned char alpn[] = { 8, 'o', 's', 's', 'l', 't', 'e', 's', 't' };

if (SSL_select_next_proto((unsigned char **)out, outlen, alpn, sizeof(alpn),
in, inlen) != OPENSSL_NPN_NEGOTIATED)
Expand Down

0 comments on commit ca9ef8e

Please sign in to comment.