1618739 - nss:tls-server: ASSERT: temp == PR_TRUE || temp == PR_FALSE, at ../../lib/ssl/ssl3exthandle.c:1059

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Kevin Jacobs [:kjacobs]]

oss-fuzz has reported a new assertion crash:

	Assertion failure: temp == PR_TRUE || temp == PR_FALSE, at ../../lib/ssl/ssl3exthandle.c:1059
	==1== ERROR: libFuzzer: deadly signal
	    #0 0x4a2331 in __sanitizer_print_stack_trace /src/llvm-project/compiler-rt/lib/asan/asan_stack.cpp:86:3
	    #1 0xa95b9d in fuzzer::PrintStackTrace() /src/libfuzzer/FuzzerUtil.cpp:205:5
	    #2 0xa55bee in fuzzer::Fuzzer::CrashCallback() /src/libfuzzer/FuzzerLoop.cpp:232:3
	    #0 0x7f83f805038f in libpthread.so.0
	    #4 0x7f83f7aa6427 in gsignal /build/glibc-LK5gWL/glibc-2.23/signal/../sysdeps/unix/sysv/linux/raise.c:54
	    #5 0x7f83f7aa8029 in abort /build/glibc-LK5gWL/glibc-2.23/stdlib/abort.c:89
	    #6 0xb0a393 in PR_Assert nspr/Debug/pr/src/io/../../../../pr/src/io/prlog.c:571:5
	    #7 0x5f2284 in ssl_ParseSessionTicket nss/out/Debug/../../lib/ssl/ssl3exthandle.c:1059:5
	    #8 0x5f105c in ssl3_ProcessSessionTicketCommon nss/out/Debug/../../lib/ssl/ssl3exthandle.c:1232:10
	    #9 0x5887c7 in tls13_ServerHandlePreSharedKeyXtn nss/out/Debug/../../lib/ssl/tls13exthandle.c:539:18
	    #10 0x5ea536 in ssl_CallExtensionHandler nss/out/Debug/../../lib/ssl/ssl3ext.c:424:22
	    #11 0x5e9ef2 in ssl3_HandleParsedExtensions nss/out/Debug/../../lib/ssl/ssl3ext.c:552:14
	    #12 0x5c664f in ssl3_HandleClientHello nss/out/Debug/../../lib/ssl/ssl3con.c:8771:10
	    #13 0x5c4f93 in ssl3_HandleHandshakeMessage nss/out/Debug/../../lib/ssl/ssl3con.c:12085:18
	    #14 0x5cb610 in ssl3_HandleHandshake nss/out/Debug/../../lib/ssl/ssl3con.c:12304:22

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20933

Regression window points to TLS 1.3 enablement: https://hg.mozilla.org/projects/nss/log?rev=7f17b911ac990a16d9bbe9edea0b52cbaa3642e2%3A%3Abc77cf318f388f55790b99d5f23a9c1f2bd9f900&revcount=10000

Reported: 2020-02-27
Disclosure: 2020-05-27

Comment 1

[Kevin Jacobs [:kjacobs]]

This is not really a security bug. Under normal execution, this code would be hit after decrypting (which includes a MAC check) on the session ticket. In fuzzer builds, the encryption and integrity check is a no-op [1]. Since the server is the producer and the consumer (and encryptor) of these tickets, it's essentially a case of the server attacking itself. The end result is a PRbool (typedef int) with an invalid value that still evaluates to true where expected.

Ifdefing the assertion or just making it an actual runtime check are both valid fixes.

[1] https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/selfencrypt.c#80-94

Comment 2

[Kevin Jacobs [:kjacobs]]

Attachment: Bug 1618739 - Don't assert fuzzer behavior in SSL_ParseSessionTicket

Comment 3

[J.C. Jones [:jcj] (he/they)]

Agreed, not a real security issue. Probably can just open this; I can do that if you agree. Thanks, Kevin

Comment 4

[Kevin Jacobs [:kjacobs]]

Yes. Thanks for the second opinion, feel free to open.

Comment 5

[Kevin Jacobs [:kjacobs]]

https://hg.mozilla.org/projects/nss/rev/12fc91fad84ad7f514d7abe64c15b375515a3710