Closed Bug 892017 Opened 6 years ago Closed 6 years ago

ASan: xpcshell test security/manager/ssl/tests/unit/test_ocsp_stapling.js triggers stack-buffer-overflow

Categories

(Core :: Security: PSM, defect, major)

x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: decoder, Assigned: keeler)

References

(Blocks 1 open bug)

Details

(Keywords: sec-want, Whiteboard: [asan][asan-test-failure])

Attachments

(1 file, 1 obsolete file)

The xpcshell test security/manager/ssl/tests/unit/test_ocsp_stapling.js triggers a stack-buffer-overflow in ASan builds, m-c revision 5769b3e238ff. It seems to originate from the OCSPStaplingServer:


=================================================================
==20489==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff39753e98 at pc 0x7fd8bbd91561 bp 0x7fff39753c50 sp 0x7fff39753c48
READ of size 4 at 0x7fff39753e98 thread T0
#0 0x7fd8bbd91560 in PORT_ArenaAlloc_Util security/nss/lib/util/secport.c:244
#1 0x7fd8bbd915e7 in PORT_ArenaZAlloc_Util security/nss/lib/util/secport.c:279
#2 0x7fd8bbe2b14a in ocsp_CreateCertStatus security/nss/lib/certhigh/ocspsig.c:49
#3 0x44324e in GetOCSPResponseForType(OCSPStapleResponseType, CERTCertificateStr*, PLArenaPool*) security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:277
#4 0x4443ce in DoSNISocketConfig(PRFileDesc*, SECItemStr const*, unsigned int, void*) security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:459
#5 0x7fd8bc4443a7 in ssl3_HandleClientHello security/nss/lib/ssl/ssl3con.c:7653
#6 0x7fd8bc449421 in ssl3_HandleHandshake security/nss/lib/ssl/ssl3con.c:10386
#7 0x7fd8bc455211 in ssl3_GatherCompleteHandshake security/nss/lib/ssl/ssl3gthr.c:366
#8 0x7fd8bc45715e in ssl_GatherRecord1stHandshake security/nss/lib/ssl/sslcon.c:1226
#9 0x7fd8bc471df3 in ssl_Do1stHandshake security/nss/lib/ssl/sslsecur.c:119
#10 0x7fd8bc475b69 in ssl_SecureRecv security/nss/lib/ssl/sslsecur.c:1133
#11 0x7fd8bc48ce96 in ssl_Recv security/nss/lib/ssl/sslsock.c:2118
#12 0x4426ac in ReadRequest(Connection*) security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:190
#13 0x4428e0 in HandleConnection(PRFileDesc*, PRFileDesc*) security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:209
#14 0x444dca in StartServer() security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:549
#15 0x445235 in main security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:586
#16 0x7fd8bad9876c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
#17 0x441fac in _start ??:0
Address 0x7fff39753e98 is located in stack of thread T0 at offset 88 in frame
#0 0x4440bf in DoSNISocketConfig(PRFileDesc*, SECItemStr const*, unsigned int, void*) security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp:420
This frame has 1 object(s):
[32, 88) 'arena'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
Shadow bytes around the buggy address:
0x1000672e2780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000672e2790: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000672e27a0: 00 00 00 00 f1 f1 f1 f1 00 f4 f4 f4 f2 f2 f2 f2
0x1000672e27b0: 00 f4 f4 f4 f3 f3 f3 f3 00 00 00 00 00 00 00 00
0x1000672e27c0: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
=>0x1000672e27d0: 00 00 00[f4]f3 f3 f3 f3 00 00 00 00 00 00 00 00
0x1000672e27e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000672e27f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000672e2800: f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 04 f4 f4 f4
0x1000672e2810: f2 f2 f2 f2 00 04 f4 f4 f2 f2 f2 f2 00 00 00 f4
0x1000672e2820: f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2 04 f4 f4 f4
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
Heap right redzone:    fb
Freed heap region:     fd
Stack left redzone:    f1
Stack mid redzone:     f2
Stack right redzone:   f3
Stack partial redzone: f4
Stack after return:    f5
Stack use after scope: f8
Global redzone:        f9
Global init order:     f6
Poisoned by user:      f7
ASan internal:         fe
==20489==ABORTING


Not marking s-s because this looks like a test-only failure.
Attached patch patch (obsolete) — Splinter Review
Looks like I was using PLArenaPool incorrectly.
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attachment #773632 - Flags: review?(brian)
Comment on attachment 773632 [details] [diff] [review]
patch

Review of attachment 773632 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/manager/ssl/tests/unit/test_ocsp_stapling/OCSPStaplingServer.cpp
@@ +452,5 @@
>      PrintPRError("SSL_ConfigSecureServer failed");
>      return SSL_SNI_SEND_ALERT;
>    }
>  
> +  PLArenaPool *arena = PORT_NewArena(1024);

You need to check the return value of this function because it could fail.
Attachment #773632 - Flags: review?(brian) → review-
Attached patch patch v2Splinter Review
Thanks, Brian. Added a check that PORT_NewArena succeeded.
Attachment #773632 - Attachment is obsolete: true
Attachment #778598 - Flags: review?(brian)
Attachment #778598 - Flags: review?(brian) → review+
https://hg.mozilla.org/mozilla-central/rev/6b41552ecce1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.