Closed Bug 1577953 (CVE-2019-11759) Opened 1 year ago Closed 1 year ago

HKDF SHA1 stack buffer overflow (write)

Categories

(NSS :: Libraries, defect, P1)

defect

Tracking

(firefox-esr60 wontfix, firefox-esr6870+ fixed, firefox67 wontfix, firefox68 wontfix, firefox69- wontfix, firefox70+ fixed, firefox71+ fixed)

RESOLVED FIXED
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 70+ fixed
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 - wontfix
firefox70 + fixed
firefox71 + fixed

People

(Reporter: guidovranken, Assigned: kjacobs)

References

Details

(4 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main70+][adv-esr68.2+][post-critsmash-triage])

Attachments

(4 files, 1 obsolete file)

=================================================================
==31990==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc26494f80 at pc 0x000000afd696 bp 0x7ffc26494910 sp 0x7ffc26494908
WRITE of size 4 at 0x7ffc26494f80 thread T0
#0 0xafd695 in SHA1_End /home/jhg/nss/nss/out/Debug/../../lib/freebl/sha_fast.c:174:5
#1 0x97e8ce in HMAC_Finish /home/jhg/nss/nss/out/Debug/../../lib/freebl/alghmac.c:133:5
#2 0x68fcca in NSC_DeriveKey /home/jhg/nss/nss/out/Debug/../../lib/softoken/pkcs11c.c:7951:21
#3 0x56b033 in PK11_DeriveWithTemplate /home/jhg/nss/nss/out/Debug/../../lib/pk11wrap/pk11skey.c:1609:15
#4 0x569c3f in PK11_Derive /home/jhg/nss/nss/out/Debug/../../lib/pk11wrap/pk11skey.c:1465:12
#5 0x5192eb in main /home/jhg/nss/cryptofuzz/poc_hkdf_sha1_oob_write.cpp:38:28
#6 0x7f169494eb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#7 0x41c729 in _start (/home/jhg/nss/cryptofuzz/a.out+0x41c729)

Address 0x7ffc26494f80 is located in stack of thread T0 at offset 800 in frame
#0 0x681abf in NSC_DeriveKey /home/jhg/nss/nss/out/Debug/../../lib/softoken/pkcs11c.c:6504

This frame has 34 object(s):
[32, 40) 'crv' (line 6519)
[64, 65) 'cktrue' (line 6520)
[80, 88) 'keyType' (line 6521)
[112, 120) 'classType' (line 6522)
[144, 148) 'outLen' (line 6530)
[160, 180) 'sha_out' (line 6531)
[224, 800) 'key_block' (line 6532) <== Memory access at offset 800 overflows this variable
[928, 992) 'crsrdata' (line 6711)
[1024, 1048) 'crsr' (line 6793)
[1088, 1112) 'master' (line 6794)
[1152, 1176) 'pms' (line 6795)
[1216, 1240) 'pms231' (line 6877)
[1280, 1304) 'seed' (line 6878)
[1344, 1368) 'master232' (line 6879)
[1408, 1472) 'srcrdata' (line 6978)
[1504, 1568) 'crsrdata310' (line 6979)
[1600, 1624) 'srcr' (line 7075)
[1664, 1688) 'keyblk' (line 7076)
[1728, 1752) 'master375' (line 7077)
[1792, 1816) 'des3key' (line 7211)
[1856, 1880) 'derived' (line 7659)
[1920, 1944) 'dhPublic' (line 7659)
[1984, 2008) 'dhPrime' (line 7660)
[2048, 2072) 'dhSubPrime' (line 7660)
[2112, 2136) 'dhValue' (line 7660)
[2176, 2200) 'ecScalar' (line 7707)
[2240, 2264) 'ecPoint' (line 7707)
[2304, 2328) 'tmp' (line 7708)
[2368, 2376) 'secret' (line 7710)
[2400, 2424) 'newPoint' (line 7742)
[2464, 2528) 'hashbuf' (line 7863)
[2560, 2564) 'bufLen' (line 7897)
[2576, 2577) 'bi' (line 7934)
[2592, 2596) 'len1088' (line 7942)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions are supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /home/jhg/nss/nss/out/Debug/../../lib/freebl/sha_fast.c:174:5 in SHA1_End
Shadow bytes around the buggy address:
0x100004c8a9a0: 00 00 04 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00
0x100004c8a9b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100004c8a9c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100004c8a9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100004c8a9e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100004c8a9f0:[f2]f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2
0x100004c8aa00: f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f8 f2
0x100004c8aa10: f2 f2 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8 f8 f2
0x100004c8aa20: f2 f2 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8 f8 f2
0x100004c8aa30: f2 f2 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8 f8 f8
0x100004c8aa40: f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8
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
==31990==ABORTING

Flags: sec-bounty?

I cannot compile and test Firefox right now, but it looks like this might be accessible through WebCrypto

Firefox confirmed vulnerable.

Assignee: nobody → nobody
Group: firefox-core-security → crypto-core-security
Component: Security → Libraries
Product: Firefox → NSS
QA Contact: jjones
Version: unspecified → other
Assignee: nobody → kjacobs.bugzilla
Severity: normal → critical
Status: UNCONFIRMED → ASSIGNED
Type: task → defect
Ever confirmed: true
Priority: -- → P1
Attachment #9091567 - Attachment description: Bug 1577953 - Allocate space for longer HKDF expensions when necessary → Bug 1577953 - Allocate space for longer HKDF expansions when necessary

I'm downgrading this to sec-moderate. Because of a separate (functional) issue in checking the max output length, the overflow is limited to four bytes of pseudorandom data.

Patch incoming, just adding some additional tests for HKDF.

Keywords: sec-highsec-low
Keywords: sec-lowsec-moderate
Attached file Bug 1577953 - Add HKDF test vectors (obsolete) —

Adds test vectors for SHA1/256/512 HKDF. This includes the RFC test vectors, as well as upper-bound length checks for the output key material.

(In reply to Kevin Jacobs [:kjacobs] from comment #4)

I'm downgrading this to sec-moderate. Because of a separate (functional) issue in checking the max output length, the overflow is limited to four bytes of random data.

What do you mean by "random data"? If the AddressSanitizer report is correct, a stack field is overwritten. How the stack is laid out is determined by the compiler and can differ across compiler/NSS versions.

There are over a dozen pointer variables in NSC_DeriveKey that if (partially) overwritten could result in more serious issues.

Flags: needinfo?(kjacobs.bugzilla)

Ping

The data that overwrites is the output of the HMAC, which isn't random, though it's only controllable by trial-and-error.

Going by the definitions for the security severity ratings [1], I think this one is pretty split between sec-high and sec-moderate and would need real effort to determine exactly how useful this would be as a widget. The primary pusher for me to sec-moderate is that it's at the end of the function, so whatever stack overwrite happens needs to do its job quickly. However, there are opportunities for that (particularly during sftk_DestroyObject), so it's far from cut-and-dry.

Since it could go either way here, I'm going to err on the side of caution here and re-assign this to sec-high.

I really do appreciate your attention to these bugs and reporting them, Guido. Thank you for the continued help! We appreciate it a lot - keep up the good work. Sorry for the delays here, I've been traveling.

[1] https://wiki.mozilla.org/Security_Severity_Ratings

Flags: needinfo?(kjacobs.bugzilla)
Keywords: sec-moderatesec-high

[Tracking Requested - why for this release]:
We'll want to collect this fix into a point release of NSS 3.44 for ESR and NSS 3.46 for Beta. I'll start the tracking bugs.

Comment on attachment 9091567 [details]
Bug 1577953 - Support longer (up to RFC maximum) HKDF outputs

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: It's easy to cause the effect, but it's not clear how easy this widget would be, as it overwrites four bytes within the stack which are unlikely to be important, but it's not been analyzed on all platforms to be sure that they're unimportant.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which older supported branches are affected by this flaw?: All
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: We're planning simultaneous releases of NSS for Firefox 70 Beta and for Firefox 68 ESR, with a landing into Firefox Nightly at the same time. Release Management has requested landing dates of 2 October for compat testing (https://bugzilla.mozilla.org/show_bug.cgi?id=1581998#c2).
  • How likely is this patch to cause regressions; how much testing does it need?: Very low chance of regressions.
Attachment #9091567 - Flags: sec-approval?

sec-approval+ for trunk. We'll want beta and ESR68 patches as well to match the requests made by release management.

Attachment #9091567 - Flags: sec-approval? → sec-approval+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019]
Attachment #9091567 - Attachment description: Bug 1577953 - Allocate space for longer HKDF expansions when necessary → Bug 1577953 - Support longer (up to RFC maximum) HKDF outputs

Re exploitability, for what it's worth: I just clicked my proof of concept and got a "Gah. Your tab just crashed." screen. Ubuntu 16.04.6 LTS, Firefox 69.0.1 (Ubuntu-provided package), x64.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 3.47
Blocks: 1585429

Comment on attachment 9091889 [details]
Bug 1577953 - Add HKDF test vectors

Revision D45434 was moved to bug 1585429. Setting attachment 9091889 [details] to obsolete.

Attachment #9091889 - Attachment is obsolete: true
Group: crypto-core-security → core-security-release

Some things are not entirely clear to me:

-> Firefox 69.0.2 landed after the fix was committed, but the changelog doesn't mention it: https://www.mozilla.org/en-US/firefox/69.0.2/releasenotes/ . Will it be included in the next release?
-> I'd like to add this and the other NSS bugs I found to my public Cryptofuzz Hall of Fame (https://github.com/guidovranken/cryptofuzz#hall-of-fame). Now that the fixes have been committed, can I mention the bugs publicly or should I wait until the Bugzilla reports are made public?
-> Now that the bug(s) have been confirmed, fixed and their severity established, will someone be contacting me about the bug bounty?

Thanks

Flags: sec-bounty? → sec-bounty+

It will be in Firefox 70, currently Firefox Beta. It missed 70b12 last week (narrowly) but it was in today's 70b13 build. It will also be in the next release of Firefox ESR, coming out on 22 October.

I think we would prefer to wait to disclose until Firefox 70 is released on 22 October - Dan, can you check me on that?

Flags: needinfo?(dveditz)

^

Flags: needinfo?(dveditz) → needinfo?(tom)

Thanks JC.

(In reply to Guido Vranken from comment #15)

-> I'd like to add this and the other NSS bugs I found to my public Cryptofuzz Hall of Fame (https://github.com/guidovranken/cryptofuzz#hall-of-fame). Now that the fixes have been committed, can I mention the bugs publicly or should I wait until the Bugzilla reports are made public?

We would prefer that details of the issue not be published until this bugzilla bug becomes public. Typically that happens shortly after the release of the version with the fix.

-> Now that the bug(s) have been confirmed, fixed and their severity established, will someone be contacting me about the bug bounty?

You should have gotten an email from me yesterday. =)

Flags: needinfo?(tom)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019] → [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019][adv-main70+]
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019][adv-main70+] → [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019][adv-main70+][adv-esr68.2+]
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019][adv-main70+][adv-esr68.2+] → [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019][adv-main70+][adv-esr68.2+][post-critsmash-triage]
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [land 2 Oct 2019][adv-main70+][adv-esr68.2+][post-critsmash-triage] → [reporter-external] [client-bounty-form] [verif?][adv-main70+][adv-esr68.2+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.