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 ...
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)
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] 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.
With a very large number the aCurver pointer will overflow or missed i something?
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.
https://hg.mozilla.org/mozilla-central/rev/8276088e9fec Not sure how far back we want to backport this.
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
Paul, any reason why you don't request an uplift to esr31? merci!
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)
Attachment #8466655 - Flags: approval-mozilla-esr31?
Attachment #8466655 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Comment on attachment 8466655 [details] [diff] [review] r= Per comment 9.
Attachment #8466655 - Flags: approval-mozilla-b2g30?
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.
Attachment #8466655 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
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/
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.
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-
You need to log in before you can comment on or make changes to this bug.