Bug 778603 (CVE-2012-4204)

Out of bounds read in str_unescape

RESOLVED FIXED in Firefox 17

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: Scott Bell, Assigned: luke)

Tracking

({crash, regression, sec-high})

Trunk
mozilla17
x86
All
crash, regression, sec-high
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox14 affected, firefox15 affected, firefox16 affected, firefox17 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [adv-track-main17+])

Attachments

(6 attachments)

(Reporter)

Description

5 years ago
I ran into this issue while fuzzing.  ASAN reports this as a heap overflow.  I wasn't able to symbolize the ASAN output for some unknown reason.  However, I see it crashing at strange locations on different runs on optimized builds.

In debug builds, it hits this assertion and crashes @ NULL: 
Assertion failure: mLength + 1 <= mReserved, at ./../../dist/include/js/Vector.h:788

Windbg references this code: mozjs!str_unescape+0x23e [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsstr.cpp @ 352]

I have attached some logs from both debug and optimized builds.
(Reporter)

Comment 1

5 years ago
Created attachment 647033 [details]
ASAN Debug Build Output
(Reporter)

Comment 2

5 years ago
Created attachment 647034 [details]
ASAN Optimized Build Output
(Reporter)

Comment 3

5 years ago
Created attachment 647035 [details]
Windbg Debug Build Output
(Reporter)

Comment 4

5 years ago
Created attachment 647036 [details]
Windbg Optimized Build Output
(Reporter)

Comment 5

5 years ago
Created attachment 647038 [details]
Repro File
Assignee: nobody → general
Component: Untriaged → JavaScript Engine
Keywords: sec-high
Product: Firefox → Core
Summary: Crash @ mozjs!str_unescape+0x23e → Crash in str_unescape
(Assignee)

Comment 6

5 years ago
Thanks for reporting!  The test-case reproduces in the shell.  A debug build asserts early reporting a call to infallibleAppend that has not reserved enough space.

Bug 708873 seems to be at fault; unfortunately the author is gone and the reviewer is on vacation.  I guess I'll look at it.

(cc'ing Jesse for fuzzer fodder.)
Blocks: 708873
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 7

5 years ago
Created attachment 648015 [details] [diff] [review]
fix

Oh my, the bug is much simpler than that: there is an unsigned underflow in the expression (k > length - 6) when length < 6.
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #648015 - Flags: review?(dmandelin)
Attachment #648015 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 8

5 years ago
Reduced testcase:
  unescape("0%u0000".substr(0,2))

Unfortunately, bug 708873 went out with FF11.
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
status-firefox-esr10: --- → unaffected
Per luke's request, I landed this fix on m-c inside an inbound merge commit rather than with its own changeset. You can confirm that the fix is present there.
https://hg.mozilla.org/mozilla-central/rev/61d7f15ea6a7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
status-firefox17: affected → fixed
(Assignee)

Updated

5 years ago
Blocks: 779863
Keywords: verifyme
Whiteboard: [adv-track-main17+]
Alias: CVE-2012-4204
Flags: sec-bounty?
This bug was marked sec-high early before we understood the nature of the vulnerability. Understanding what the bug is now (out of bounds read), is that still appropriate? Will any part of this code write into an area assuming there's enough space because of the bogus length or it is only the read path that goes wrong?
Flags: needinfo?(luke)
Keywords: regression
(Assignee)

Comment 11

4 years ago
IIRC, there was the possibility of writing past the end of the buffer (infallibleAppend is being used), so this sec-high sounds appropriate.
Flags: needinfo?(luke)
Flags: sec-bounty? → sec-bounty+
Group: core-security
Keywords: crash
Summary: Crash in str_unescape → Out of bounds read in str_unescape
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.