1047831 - (CVE-2014-1565) Out-of-bounds Read in mozilla::dom::AudioEventTimeline<mozilla::ErrorResult>::ExtractValueFromCurve

Status: VERIFIED FIXED

(Core :: Web Audio, defect)

Description

[Holger Fuhrmannek]

Attachment: 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

...

Comment 1

[Paul Adenot (:padenot)]

Attachment: r=

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.

Comment 2

[Daniel Veditz [:dveditz]]

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 3

[Paul Adenot (:padenot)]

Comment on attachment 8466655 [details] [diff] [review]
r=

[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.

Comment 4

[Holger Fuhrmannek]

With a very large number the aCurver pointer will overflow or missed i something?

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8466655 [details] [diff] [review]
r=

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.

Comment 6

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8276088e9fec

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8276088e9fec

Not sure how far back we want to backport this.

Comment 8

[Paul Adenot (:padenot)]

Pretty far.

Comment 9

[Paul Adenot (:padenot)]

Comment on attachment 8466655 [details] [diff] [review]
r=

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

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Paul, any reason why you don't request an uplift to esr31? merci!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bef1ccd4c38d
https://hg.mozilla.org/releases/mozilla-beta/rev/0c488a1d2142

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/0c488a1d2142

Comment 13

[Paul Adenot (:padenot)]

Comment on attachment 8466655 [details] [diff] [review]
r=

(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)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr31/rev/76cae6d18b73

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8466655 [details] [diff] [review]
r=

Per comment 9.

Comment 16

[Wayne Chang [:wchang]]

Comment on attachment 8466655 [details] [diff] [review]
r=

From comments and approval request details, I think we'll take/need this on 1.4 as well.
Thanks.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/07d78d0f9bef

Comment 18

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue several times using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1406975005/

fx-34 m-c: [PASSED]
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1408701721/

fx-33 aurora: [PASSED]
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1408715515/

fx-32 beta: [PASSED]
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux64-asan/1408673354/

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:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/

Comment 19

[Kamil Jozwiak [:kjozwiak]]

fx 31.1.0esrpre: [PASSED]
- Built from: http://hg.mozilla.org/releases/mozilla-esr31/rev/d736074cabbf

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.

Comment 20

[chris hofmann]

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.

Comment 21

[Al Billings [:abillings - ex-MoCo]]

Minusing this for bounty because it is a debug only issue and may not even be a moderate security issue.