Closed Bug 1207667 Opened 7 years ago Closed 7 years ago

heap-buffer-overwrite in BrotliFileInputFunction

Categories

(Core :: Networking: HTTP, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
firefox43 --- unaffected
firefox44 + fixed
firefox-esr38 --- unaffected

People

(Reporter: tsmith, Unassigned)

References

(Blocks 1 open bug)

Details

(4 keywords)

Attachments

(2 files, 2 obsolete files)

Not sure which component this should go under so I copied bug 366559 for now.

I am fuzzing the latest version of the code from:
https://github.com/google/brotli
Attached file valgrid_output.txt (obsolete) —
Attached file test_case.compressed (obsolete) —
We are looking at this
I could reproduce this on the github version, and confirmed that it is already fixed internally. We will push a new decoder to google/brotli repo soon.
I updated https://github.com/google/brotli, could you verify that it is fixed?
Flags: needinfo?(twsmith)
Group: core-security → network-core-security
Attached file valgrid_output.txt
I am still able to reproduce this issue.

I did a git pull and I am not at https://github.com/google/brotli/commit/24dca87f96dcd29a2803ffa697ea1d823d1ea703.
Attachment #8664955 - Attachment is obsolete: true
Flags: needinfo?(twsmith)
Attached file test_case.compressed
updated test case
Attachment #8664956 - Attachment is obsolete: true
(In reply to Tyson Smith [:tsmith] from comment #6)
> Created attachment 8665675 [details]
> valgrid_output.txt
> 
> I am still able to reproduce this issue.
> 
> I did a git pull and I am not at
> https://github.com/google/brotli/commit/
> 24dca87f96dcd29a2803ffa697ea1d823d1ea703.

Typo :)
I am now at https://github.com/google/brotli/commit/24dca87f96dcd29a2803ffa697ea1d823d1ea703
We fixed one more bug in decoder, the new test case is fixed in the latest commit:
https://github.com/google/brotli/commit/6dd53d618377e0efd3b0d2de9f03c284ccf81695
That seems to have fixed the bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
This bug should remain open until the upstream fix has landed on mozilla-central.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
[Tracking Requested - why for this release]:
Keywords: sec-critical
I can verify that cset from github 933bb9bd800c8f5f7f6a02382d33c902a98ef73a makes valgrind pass the testcase. I can also verify that the cset we are shipping on >=43 for woff2 also passes the testcase.
bug 1207298 checked in a library update that  will also resolve this issue.
Group: network-core-security → core-security-release
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
I'm clearing the ni flag because I don't see a question to be answered.
Flags: needinfo?(mcmanus)
(In reply to Patrick McManus [:mcmanus] from comment #17)
> I'm clearing the ni flag because I don't see a question to be answered.

Sorry about that. :)
Tyson, can you verify fixed? Thanks!
Flags: needinfo?(twsmith)
Status: RESOLVED → VERIFIED
Flags: needinfo?(twsmith)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.