Closed Bug 1047831 (CVE-2014-1565) Opened 10 years ago Closed 10 years ago

Out-of-bounds Read in mozilla::dom::AudioEventTimeline<mozilla::ErrorResult>::ExtractValueFromCurve


(Core :: Web Audio, defect)

34 Branch
Not set



Tracking Status
firefox31 --- wontfix
firefox32 --- verified
firefox33 --- verified
firefox34 --- verified
firefox-esr24 --- unaffected
firefox-esr31 --- verified
b2g-v1.3 --- wontfix
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed


(Reporter: hofusec, Assigned: padenot)


(4 keywords, Whiteboard: [adv-main32+][adv-esr31.1+])


(2 files)

Attached file testcase.html
Stacktrace: (from FatalSignalHandler):

Assertion failure: ratio >= 0.0 (Ratio can never be negative here), at ../../../dist/include/AudioEventTimeline.h:425

#5  mozilla::dom::AudioEventTimeline<mozilla::ErrorResult>::ExtractValueFromCurve (startTime=<optimized out>, 
    aCurve=<optimized out>, aCurveLength=<optimized out>, duration=<optimized out>, t=<optimized out>)
    at ../../../dist/include/AudioEventTimeline.h:425

#6  0x00007f04e6ecfdee in mozilla::dom::AudioEventTimeline<mozilla::ErrorResult>::GetValueAtTime<double> (
    this=0x60c0003115e0, aTime=-nan(0xfffffffffffff)) at ../../../dist/include/AudioEventTimeline.h:258
#7  0x00007f04e6ebf8a7 in mozilla::dom::BiquadFilterNode::GetFrequencyResponse (this=0x610000153e40, 
    aFrequencyHz=..., aMagResponse=..., aPhaseResponse=...)
    at /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/content/media/webaudio/BiquadFilterNode.cpp:338
#8  0x00007f04e55f777c in mozilla::dom::BiquadFilterNodeBinding::getFrequencyResponse (cx=0x615000ac5b00, 
    self=<optimized out>, args=..., obj=...) at ./BiquadFilterNodeBinding.cpp:256

Severity: normal → critical
Ever confirmed: true
Keywords: crash, testcase
Assignee: nobody → paul
Attached patch r=Splinter Review
This happens because `TimesEqual` is fuzzy.

Then we compare aTime (which is 0) and the start of the curve (which is
0.0000000001), we decide it's close enough that we need to extract the value
from the curve, and then we go:

> (0 - 0.0000000001) / duration

that is negative and crashes on the assert. It seems the right thing to do to
simply clamp the lower bound to zero, here.
Attachment #8466655 - Flags: review?(ehsan)
Attachment #8466655 - Flags: review?(ehsan) → review+
So in an opt build the negative ratio multiplied by the curve length is negative, and is then cast to a very large number. This would read random values out of high memory
Comment on attachment 8466655 [details] [diff] [review]

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Probably hard, this is fairly hard to control.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
I specifically did not put any comment.

Which older supported branches are affected by this flaw?
I believe all version from Firefox 25

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Very similar, this has not been touched in ages.

How likely is this patch to cause regressions; how much testing does it need?
Not likely, this is super edge-casey. Just pushing to try should be good.
Attachment #8466655 - Flags: sec-approval?
With a very large number the aCurver pointer will overflow or missed i something?
Comment on attachment 8466655 [details] [diff] [review]

As a sec-moderate, this doesn't need sec-approval. That is only necessary for critical and high rated issues. Please go ahead and check in.
Attachment #8466655 - Flags: sec-approval?

Not sure how far back we want to backport this.
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Pretty far.
Comment on attachment 8466655 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: initial web audio landing (firefox 25)
[User impact if declined]: crash
[Describe test coverage new/current, TBPL]: I converted the test case into a crash test, not landed for now
[Risks and why]: this is not risky at all, the cause and fix are well understood
[String/UUID change made/needed]: none
Attachment #8466655 - Flags: approval-mozilla-beta?
Attachment #8466655 - Flags: approval-mozilla-aurora?
Paul, any reason why you don't request an uplift to esr31? merci!
Flags: needinfo?(paul)
Attachment #8466655 - Flags: approval-mozilla-beta?
Attachment #8466655 - Flags: approval-mozilla-beta+
Attachment #8466655 - Flags: approval-mozilla-aurora?
Attachment #8466655 - Flags: approval-mozilla-aurora+
Comment on attachment 8466655 [details] [diff] [review]

(In reply to Sylvestre Ledru [:sylvestre] from comment #10)
> Paul, any reason why you don't request an uplift to esr31? merci!

No idea, I must have missed it.

[Approval Request Comment]
(see comment 9)
Attachment #8466655 - Flags: approval-mozilla-esr31?
Flags: needinfo?(paul)
Attachment #8466655 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Comment on attachment 8466655 [details] [diff] [review]

Per comment 9.
Attachment #8466655 - Flags: approval-mozilla-b2g30?
QA Whiteboard: [qa+]
Flags: needinfo?(wchang)
Comment on attachment 8466655 [details] [diff] [review]

From comments and approval request details, I think we'll take/need this on 1.4 as well.
Attachment #8466655 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Flags: needinfo?(wchang)
Reproduced the original issue several times using the following build:

fx-34 m-c: [PASSED]

fx-33 aurora: [PASSED]

fx-32 beta: [PASSED]

Test Cases used against all of the above channels:

- opened the poc from comment #0 in 10 different tabs
- opened the poc from comment #0 in 10 different windows
- opened the poc from comment #0 in 10 different private tabs
- opened the poc from comment #0 in 10 different private windows
- opened the poc from comment #0 in 10 different e10s tabs
- opened the poc from comment #0 in 10 differnet e10s windows

Only thing remaining is firefox-esr31, will need to create my own asan build. Looks like they're not being created under:
fx 31.1.0esrpre: [PASSED]
- Built from:

Configure arguments:

--enable-address-sanitizer --disable-jemalloc --disable-crashreporter --disable-elf-hack --enable-debug-symbols --disable-install-strip --enable-optimize --enable-debug

Went through the test cases from comment #18 without any issues.
QA Whiteboard: [qa+] → [qa!]
Whiteboard: [adv-main32+][adv-esr31.1+]
Alias: CVE-2014-1565
Flags: sec-bounty?
just to summarize this is a possible exploitable crash in debug builds, but non-crash read values out of high memory in opt builds.  Its not clear what the exploit-ability of this in opt-builds and that's why the sec moderate rating is assigned.  does this all sound right.  trying to confirm sec-moderate and not paying a bounty for this one.
Minusing this for bounty because it is a debug only issue and may not even be a moderate security issue.
Flags: sec-bounty? → sec-bounty-
Group: core-security → core-security-release
Group: core-security-release
Flags: sec-bounty-hof+
You need to log in before you can comment on or make changes to this bug.