+++ This bug was initially created as a clone of Bug #990794 +++ If we land a fix for the size calculation in bug 990794, then that leave a DOS bug, which can be fixed by limiting the channel count. Fixing the DOS gives away more information about how to induce the integer and heap overflow, so we have the option of landing this fix separately. See also bug 987679 which can be induced through large channel counts.
5 years ago
Attachment #8400393 - Flags: review?(paul) → review+
Had to change some uint32_t array sizes/indexes to size_t due to changes from bug 1004098. https://hg.mozilla.org/integration/mozilla-inbound/rev/28f1a546126f
Comment on attachment 8400393 [details] [diff] [review] limit ChannelMergerNode output channel count [Approval Request Comment] Requesting approval for this patch as part of fixing bug 990868. Bug caused by (feature/regressing bug #): The security issue that I'm concerned about here is bug 987679, which could be triggered using WaveShaperNode.oversample since Bug 875277 landed in 26. User impact if declined: Possible sec-critical. Testing completed (on m-c, etc.): on m-i, requesting approval to land once merged to m-c. Risk to taking this patch (and alternatives if risky): low. String or IDL/UUID changes made by this patch: none.
The version to land on branches is the one reviewed here (attachment 8400393 [details] [diff] [review]), without the size_t changes on m-c/32.
Whiteboard: [don't transplant m-c changeset to branches]
Karl, does it impact ESR? Thanks
Only versions 26 and later are affected, so ESR24 is not affected.
This was merged last Friday. https://hg.mozilla.org/mozilla-central/rev/28f1a546126f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment on attachment 8400393 [details] [diff] [review] limit ChannelMergerNode output channel count sec-lows aren't being approved for the B2G release branches
Changing to sec-critical as this is part of fixing bug 991533, which is now necessary to avoid sec-critical bug 987679.
Comment on attachment 8400393 [details] [diff] [review] limit ChannelMergerNode output channel count FWIW, sec-crits don't need approval for uplift to the B2G release branches per the B2G Landing Page. https://wiki.mozilla.org/Release_Management/B2G_Landing
Does anyone have a suggestion on how to verify this sec-critical bug fix? Thanks.
(In reply to Matt Wobensmith from comment #14) > Does anyone have a suggestion on how to verify this sec-critical bug fix? The testcase in comment 990794 should no longer crash. (That's not the sec-critical part of this bug but would verify that the patch is doing what it should.)
I mean bug 990794.
Thanks Karl. On Fx30 from 2014-05-15 (pre-fix) I don't see a crash, but I do get a hang on close that requires a force quit. On Fx30/Fx31/Fx32 from 2014-06-02, no crash and program exits smoothly. I'm going to consider this verified unless there are objections.
fixed on SeaMonkey 2.26.1 (based on Gecko 29) https://hg.mozilla.org/releases/mozilla-release/rev/7a22ed0a3752
(In reply to Justin Wood (:Callek) from comment #18) > fixed on SeaMonkey 2.26.1 (based on Gecko 29) > > https://hg.mozilla.org/releases/mozilla-release/rev/7a22ed0a3752 err nope, couldn't get this to apply + pass tests, and that rev is a different bug
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.