Bug 1246742 (CVE-2016-1968)

Security: Buffer overflow in Brotli decompression

RESOLVED FIXED

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: lukezli, Unassigned)

Tracking

({csectype-bounds, sec-high})

Trunk
csectype-bounds, sec-high
Points:
---
Bug Flags:
sec-bounty +
qe-verify -

Firefox Tracking Flags

(firefox45 fixed, firefox46 fixed, firefox-esr38 disabled, firefox-esr45 fixed)

Details

(Whiteboard: [post-critsmash-triage][adv-main45+] merged into bug that included upstream patch for this issue (see comment 2))

Attachments

(1 attachment)

20 bytes, application/octet-stream
Details
(Reporter)

Description

3 years ago
Posted file crash.compressed
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.97 Safari/537.36

Steps to reproduce:

Using AFL, I discovered a pointer underflow bug in brotli decompression that leads to a buffer overflow on the heap.

Reproduction steps:

git clone https://github.com/google/brotli.git
cd brotli
git checkout d4f0cb98411491ade114158f363d8693e89ab473
cd tools
make -j 
setarch x86_64 -R  ./bro --decompress --input  crash.compressed  > /dev/null 

Two notes:
1. ASLR can be disabled (with setarch) to reproduce reliably. This is because pointer underflow occurs when the ringbuffer has a low-enough address space.
2. git checkout because the trunk of brotli has already been patched by the Google team. Alternatively, use Firefox's dec/ folder when making the binary to reproduce using code in Firefox's trunk.

This was introduced in Aug 2015 to brotli trunk: https://github.com/google/brotli/commit/94cd7085f79a707f5ba3d93086796e695b495975. Firefox trunk (and possibly other versions) contain the vulnerable version of the brotli source code.

I have already submitted this report to Chrome, who informed me that I can report this to you guys as well. The brotli team patched this in their trunk: https://github.com/google/brotli/commit/33aa40220b96cf95ad2b9ba61dc8d7fd2f964f2c and additionally cherry-picked this patch into Chrome 48, 49, and Chromium. Here is an example of one of their patchsets: https://codereview.chromium.org/1672793002


Actual results:

Segmentation fault due to heap buffer overflow


Expected results:

No segmentation fault

Updated

3 years ago
Group: firefox-core-security → core-security
Component: Untriaged → Networking: HTTP
Product: Firefox → Core
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1242904
(Reporter)

Comment 2

3 years ago
Is this submission still eligible for a bug bounty? I reported this to Google on 2/3 (can provide proof if necessary) but waited until today to notify Firefox since I was waiting for Google's OK to tell you guys (which I received today). I wasn't made aware that Google had already notified you on 2/5.

Please advise.
needinfo dan who helps with the bounty program,..
Flags: needinfo?(dveditz)
oh - and thanks for finding and reporting vulnerabilities!
(Reporter)

Comment 5

3 years ago
Ok awesome, thanks a lot Patrick and Dan!
(In reply to Luke Li from comment #2)
> Is this submission still eligible for a bug bounty? 

I don't know -- I've nominated so the bounty committee can discuss it.

Google is the home of this library and should already cover this with their bounty program. As seen in bug 1246742 comment 27 they let us know about security issues we need to take, just as we report stuff we found to them. On the other hand their version of this library might be in a sandboxed process resulting in a lower bounty in which case we should see if our bounty would have been higher and make up the difference.
Group: core-security → core-security-release
Flags: needinfo?(dveditz) → sec-bounty?
Whiteboard: merged into bug that included upstream patch for this issue (see comment 2)
See Also: → bug 1242904
Flags: sec-bounty? → sec-bounty+
Keywords: sec-high

Comment 7

3 years ago
Hi Luke, Thanks for reporting.  We are awarding a $3000 bounty for the notice the we needed to upgrade and the test case that works when feed to the library.  If you would like to work on a test case that shows vulnerability directly from Firefox we could add an additional bounty on top of the $3k.
Changing this from "dupe" to "fixed (depends on)" because the other bug is generic. When it comes to security bugs we should save "duplicate" for "exactly the same bug" and otherwise use depends on and set the status to FIXED. Otherwise we risk not patching things on the appropriate branches and miss crediting reporters.
Depends on: 1242904
Resolution: DUPLICATE → FIXED
Whiteboard: merged into bug that included upstream patch for this issue (see comment 2) → [adv-main45+] merged into bug that included upstream patch for this issue (see comment 2)
Alias: CVE-2016-1968
(Reporter)

Comment 9

3 years ago
Many thanks to Mozilla for the bug bounty and for the recognition. I had help with a friend, Jonathan Metzman, in discovering this vulnerability. Would it be possible to change the recognition in the security advisory to "Luke Li and Jonathan Metzman"?

Thanks again!
status-firefox45: --- → fixed
status-firefox46: --- → fixed
status-firefox-esr38: --- → disabled
status-firefox-esr45: --- → fixed
Flags: qe-verify-
Whiteboard: [adv-main45+] merged into bug that included upstream patch for this issue (see comment 2) → [post-critsmash-triage][adv-main45+] merged into bug that included upstream patch for this issue (see comment 2)
Group: core-security-release
Keywords: csectype-bounds
You need to log in before you can comment on or make changes to this bug.