1347499 - AddressSanitizer: SEGV in curve25519_64.c:209:15

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Found by oss-fuzz:

==1==ERROR: AddressSanitizer: SEGV on unknown address 0x604000030000 (pc 0x000000869f5e bp 0x7ffc18a56770 sp 0x7ffc18a56760 T0)
	==1==The signal is caused by a READ memory access.
	SCARINESS: 20 (wild-addr-read)
	#0 0x869f5d in fexpand /src/nss/lib/freebl/ecl/curve25519_64.c:209:15
	#1 0x869d25 in ec_Curve25519_mul /src/nss/lib/freebl/ecl/curve25519_64.c:507:5
	#2 0x7cea3d in ec_Curve25519_pt_mul /src/nss/lib/freebl/ecl/ecp_25519.c:118:12
	#3 0x7c02bc in ECDH_Derive /src/nss/lib/freebl/ec.c:592:16
	#4 0x74b055 in NSC_DeriveKey /src/nss/lib/softoken/pkcs11c.c:7267:18
	#5 0x6f79c7 in pk11_PubDeriveECKeyWithKDF /src/nss/lib/pk11wrap/pk11skey.c:2224:15
	#6 0x6f6fc0 in PK11_PubDeriveWithKDF /src/nss/lib/pk11wrap/pk11skey.c:2315:20
	#7 0x605b40 in ssl3_HandleECDHClientKeyExchange /src/nss/lib/ssl/ssl3ecc.c:334:11

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

This is NOT a security issue, merely a correctness issue. The affected code is here:

> /* Take a 33-byte number and expand it into polynomial form */
> static void NO_SANITIZE_ALIGNMENT
> fexpand(felem *output, const u8 *in)
> {
>     output[0] = *((const uint64_t *)(in)) & MASK51;
>     output[1] = (*((const uint64_t *)(in + 6)) >> 3) & MASK51;
>     output[2] = (*((const uint64_t *)(in + 12)) >> 6) & MASK51;
>     output[3] = (*((const uint64_t *)(in + 19)) >> 1) & MASK51;
>     output[4] = (*((const uint64_t *)(in + 25)) >> 4) & MASK51;
> }

The last line produces a pointer that is 1 byte beyond the actual input. We then take that 64-bit value, shift it by 4 bits to the right, and then ignore the 13 MSBs. So no matter what the value at in[32], we ignore it anyway.

But ASan of course doesn't like us doing that. Fixing this is easy.

Comment 2

[Franziskus Kiefer [:franziskus]]

https://hg.mozilla.org/projects/nss/rev/81a1d9f72ea5b08a95b837e9395e883ac8b9553c

Comment 3

[Ryan Sleevi]

Note there's a typo in the patch - "onlye"

Comment 4

[Franziskus Kiefer [:franziskus]]

Indeed. Fixed.
https://hg.mozilla.org/projects/nss/rev/48e2bf231184503f7c03d703f3bcd5e057d48ca3