Missing null check in s2n_connection_wipe_keys

[jouho]

Problem:

While I was debugging memory allocation issue for fuzz test, there was a null pointer error:

# ./s2n_deserialize_resumption_state_test
INFO: Seed: 4211361216
INFO: Loaded 2 modules   (105153 guards): 105058 [0x7f35c8f28988, 0x7f35c8f8f310), 95 [0x56349098c538, 0x56349098c6b4),
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#0      READ units: 1
#2      INITED cov: 2092 ft: 1227 corp: 1/1b exec/s: 0 rss: 47Mb
#3      NEW    cov: 2099 ft: 1290 corp: 2/93b exec/s: 0 rss: 48Mb L: 92/92 MS: 1 InsertRepeatedBytes-
#18     NEW    cov: 2123 ft: 1339 corp: 3/164b exec/s: 0 rss: 51Mb L: 71/92 MS: 1 InsertRepeatedBytes-
#23     NEW    cov: 2124 ft: 1355 corp: 4/166b exec/s: 0 rss: 51Mb L: 2/92 MS: 1 InsertByte-
#37     REDUCE cov: 2124 ft: 1355 corp: 4/155b exec/s: 0 rss: 53Mb L: 81/81 MS: 5 ChangeBit-CMP-InsertRepeatedBytes-ChangeBit-EraseBytes- DE: "\x01\x00"-
#38     NEW    cov: 2124 ft: 1357 corp: 5/185b exec/s: 0 rss: 54Mb L: 30/81 MS: 1 InsertRepeatedBytes-
#42     NEW    cov: 2131 ft: 1364 corp: 6/260b exec/s: 0 rss: 54Mb L: 75/81 MS: 5 InsertRepeatedBytes-PersAutoDict-InsertRepeatedBytes-ChangeByte-PersAutoDict- DE: "\x01\x00"-"\x01\x00"-
#66     NEW    cov: 2131 ft: 1366 corp: 7/329b exec/s: 0 rss: 58Mb L: 69/81 MS: 4 ChangeBit-ChangeByte-CopyPart-InsertRepeatedBytes-
#160    REDUCE cov: 2131 ft: 1366 corp: 7/322b exec/s: 0 rss: 67Mb L: 64/81 MS: 3 InsertByte-ChangeBit-EraseBytes-
#224    REDUCE cov: 2131 ft: 1366 corp: 7/307b exec/s: 0 rss: 73Mb L: 49/81 MS: 2 ChangeBinInt-EraseBytes-
#373    REDUCE cov: 2131 ft: 1366 corp: 7/289b exec/s: 0 rss: 87Mb L: 63/75 MS: 1 EraseBytes-
#389    REDUCE cov: 2131 ft: 1366 corp: 7/267b exec/s: 0 rss: 89Mb L: 27/75 MS: 2 ChangeBit-EraseBytes-
#643    REDUCE cov: 2131 ft: 1366 corp: 7/258b exec/s: 0 rss: 113Mb L: 54/75 MS: 1 EraseBytes-
#644    REDUCE cov: 2131 ft: 1366 corp: 7/241b exec/s: 0 rss: 113Mb L: 37/75 MS: 2 EraseBytes-EraseBytes-
#664    REDUCE cov: 2131 ft: 1366 corp: 7/236b exec/s: 0 rss: 115Mb L: 22/75 MS: 2 ChangeByte-EraseBytes-
#675    NEW    cov: 2142 ft: 1378 corp: 8/240b exec/s: 0 rss: 116Mb L: 4/75 MS: 3 PersAutoDict-CrossOver-ChangeByte- DE: "\x01\x00"-
#809    REDUCE cov: 2142 ft: 1378 corp: 8/233b exec/s: 0 rss: 129Mb L: 23/75 MS: 2 CMP-EraseBytes- DE: "\x00\x00\x00\x00\x00\x00\x00\x00"-
#889    REDUCE cov: 2142 ft: 1378 corp: 8/230b exec/s: 0 rss: 137Mb L: 34/75 MS: 2 ShuffleBytes-EraseBytes-
#933    REDUCE cov: 2142 ft: 1378 corp: 8/222b exec/s: 0 rss: 141Mb L: 14/75 MS: 1 EraseBytes-
#1378   REDUCE cov: 2142 ft: 1378 corp: 8/216b exec/s: 0 rss: 183Mb L: 28/75 MS: 1 EraseBytes-
#1823   REDUCE cov: 2142 ft: 1378 corp: 8/209b exec/s: 0 rss: 224Mb L: 7/75 MS: 1 EraseBytes-
#2008   REDUCE cov: 2142 ft: 1378 corp: 8/199b exec/s: 0 rss: 241Mb L: 18/75 MS: 1 EraseBytes-
#2148   REDUCE cov: 2142 ft: 1378 corp: 8/191b exec/s: 0 rss: 254Mb L: 10/75 MS: 1 EraseBytes-
#2494   REDUCE cov: 2142 ft: 1378 corp: 8/188b exec/s: 0 rss: 287Mb L: 7/75 MS: 2 InsertByte-EraseBytes-
#2504   NEW    cov: 2142 ft: 1379 corp: 9/192b exec/s: 0 rss: 288Mb L: 4/75 MS: 2 ChangeBit-ChangeBit-
#2545   REDUCE cov: 2142 ft: 1379 corp: 9/187b exec/s: 0 rss: 292Mb L: 70/70 MS: 3 ChangeBit-ShuffleBytes-EraseBytes-
#2718   REDUCE cov: 2142 ft: 1379 corp: 9/185b exec/s: 0 rss: 308Mb L: 5/70 MS: 1 EraseBytes-
#2803   REDUCE cov: 2142 ft: 1379 corp: 9/184b exec/s: 0 rss: 316Mb L: 6/70 MS: 1 EraseBytes-
#2993   REDUCE cov: 2142 ft: 1379 corp: 9/183b exec/s: 2993 rss: 334Mb L: 4/70 MS: 1 EraseBytes-
#3168   REDUCE cov: 2142 ft: 1379 corp: 9/181b exec/s: 3168 rss: 350Mb L: 4/70 MS: 1 EraseBytes-
#3363   REDUCE cov: 2142 ft: 1379 corp: 9/152b exec/s: 3363 rss: 368Mb L: 40/70 MS: 1 EraseBytes-
#3367   REDUCE cov: 2142 ft: 1379 corp: 9/136b exec/s: 3367 rss: 369Mb L: 24/70 MS: 5 EraseBytes-CrossOver-CopyPart-CMP-EraseBytes- DE: "\xc0\x09"-
#3659   REDUCE cov: 2142 ft: 1379 corp: 9/132b exec/s: 3659 rss: 396Mb L: 19/70 MS: 2 CrossOver-EraseBytes-
#3843   REDUCE cov: 2142 ft: 1379 corp: 9/131b exec/s: 3843 rss: 413Mb L: 3/70 MS: 1 EraseBytes-
#3868   NEW    cov: 2204 ft: 1442 corp: 10/143b exec/s: 3868 rss: 416Mb L: 12/70 MS: 1 CMP- DE: "\x01\x00\x00\x00\x00\x00\x00\x00"-
#3953   NEW    cov: 2213 ft: 1451 corp: 11/171b exec/s: 3953 rss: 424Mb L: 28/70 MS: 1 CrossOver-
#4039   REDUCE cov: 2213 ft: 1451 corp: 11/169b exec/s: 4039 rss: 432Mb L: 26/70 MS: 2 ChangeBit-EraseBytes-
#4063   REDUCE cov: 2213 ft: 1451 corp: 11/140b exec/s: 4063 rss: 434Mb L: 41/41 MS: 1 EraseBytes-
#4065   REDUCE cov: 2213 ft: 1451 corp: 11/129b exec/s: 4065 rss: 434Mb L: 30/30 MS: 3 EraseBytes-CMP-EraseBytes- DE: "\xff\xff\xff\xff\xff\xff\xff\xff"-
#4073   REDUCE cov: 2213 ft: 1451 corp: 11/128b exec/s: 4073 rss: 435Mb L: 2/30 MS: 1 EraseBytes-
#4099   REDUCE cov: 2213 ft: 1451 corp: 11/127b exec/s: 4099 rss: 437Mb L: 1/30 MS: 2 ChangeBinInt-EraseBytes-
/codebuild/output/src1618890001/src/github.com/aws/s2n-tls/tls/s2n_connection.c:149:5: runtime error: member access within null pointer of type 'struct s2n_connection'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /codebuild/output/src1618890001/src/github.com/aws/s2n-tls/tls/s2n_connection.c:149:5 in
AddressSanitizer:DEADLYSIGNAL
=================================================================
==5681==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000004a8 (pc 0x7f35c80f5f29 bp 0x7ffd9e3679f0 sp 0x7ffd9e367980 T0)
==5681==The signal is caused by a READ memory access.
==5681==Hint: address points to the zero page.
==5681==ERROR: AddressSanitizer failed to deallocate 0x2000 (8192) bytes at address 0x7f35c743b000
AddressSanitizer: CHECK failed: sanitizer_posix.cpp:63 "(("unable to unmap" && 0)) != (0)" (0x0, 0x0) (tid=5681)

This is because s2n_connection_wipe_keys() do not null-check the input:

s2n-tls/tls/s2n_connection.c

Lines 145 to 161 in 9738cc7

	static int s2n_connection_wipe_keys(struct s2n_connection *conn)
	{
	/* Free any server key received (we may not have completed a
	* handshake, so this may not have been free'd yet) */
	POSIX_GUARD(s2n_pkey_free(&conn->handshake_params.server_public_key));
	POSIX_GUARD(s2n_pkey_zero_init(&conn->handshake_params.server_public_key));
	POSIX_GUARD(s2n_pkey_free(&conn->handshake_params.client_public_key));
	POSIX_GUARD(s2n_pkey_zero_init(&conn->handshake_params.client_public_key));
	s2n_x509_validator_wipe(&conn->x509_validator);
	POSIX_GUARD(s2n_dh_params_free(&conn->kex_params.server_dh_params));
	POSIX_GUARD_RESULT(s2n_connection_wipe_all_keyshares(conn));
	POSIX_GUARD(s2n_kem_free(&conn->kex_params.kem_params));
	POSIX_GUARD(s2n_free(&conn->handshake_params.client_cert_chain));
	POSIX_GUARD(s2n_free(&conn->ct_response));
	return 0;
	}

Solution:

Add POSIX_ENSURE_REF at the top of the function to check for null input