382 bytes, text/html
1.67 KB, patch
|Details | Diff | Splinter Review|
1.24 KB, patch
|Details | Diff | Splinter Review|
Perhaps being a little paranoid.
Attachment #8400386 - Flags: review?(roc)
Attachment #8400386 - Flags: review?(roc) → review+
Attachment #8400387 - Flags: review?(roc) → review+
Comment on attachment 8400386 [details] [diff] [review] crash on ovrfl in SharedBuffer::Create() Please consider this a request to land both patches here. [Security approval request comment] How easily could an exploit be constructed based on the patch? There are multiple callers of the functions patched here. Creating an exploit based on the testcase here requires a little knowledge of Web Audio or looking through a fair bit of code to find this caller. Those looking at bug 877039 didn't consider this possible route. However, there may be other simpler, more obvious routes, if a large number of memory allocations first succeed. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No tests, but the patch clearly identifies the problem. Which older supported branches are affected by this flaw? The flaw goes back to 25. 24 is unaffected. If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Patches apply to 28 with minimal fuzz. How likely is this patch to cause regressions; how much testing does it need? Low risk. It is a widely used code path, with plenty of automated tests.
Attachment #8400386 - Flags: sec-approval?
Comment on attachment 8400386 [details] [diff] [review] crash on ovrfl in SharedBuffer::Create() sec-approval+ for trunk. Let's get Aurora and Beta patches created and nominated too once it is on trunk.
Attachment #8400386 - Flags: sec-approval? → sec-approval+
Comment on attachment 8400386 [details] [diff] [review] crash on ovrfl in SharedBuffer::Create() Please consider this a request for both patches here. They apply cleanly to 29/Beta. [Approval Request Comment] Bug caused by (feature/regressing bug #): Web Audio. old bug. User impact if declined: security bug. Testing completed (on m-c, etc.): This code path is used in many automated tests. Risk to taking this patch (and alternatives if risky): not risky at all. String or IDL/UUID changes made by this patch: none
Comment on attachment 8400386 [details] [diff] [review] crash on ovrfl in SharedBuffer::Create() The patches apply with fuzz 2 to 28 and b2g26_v1_2. [Approval Request Comment] Please see aurora/beta request.
I will approve the patch for aurora & beta once it has reached m-c.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/500c93ec076a https://hg.mozilla.org/releases/mozilla-aurora/rev/56d760bc26ac https://hg.mozilla.org/releases/mozilla-beta/rev/51a84afe085d https://hg.mozilla.org/releases/mozilla-beta/rev/004a7c15d761 https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/c48b469120b6 https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/abb7f1b0c6f1 https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/400b174cafc8 https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/98d412b487cf
Verifying the fix here on 32-bit builds requires comparing the crash stacks of before and after builds. The "before" build may not crash at a reliable location but it is likely to be in AudioBlockCopyChannelWithScale. The "after" build should crash in AllocateAudioBlock. 64-bit builds may now either load this testcase fine or crash on OOM. Previously they would crash like 32-bit builds.
The crashes from Windows 8.1 32bit look the same for 'before' and 'after' builds (check the crash reports bellow. Is this normal? Ubuntu 13.10 64bit: Reproduced the crash on old nightly: bp-0f842743-da20-4e71-93e1-28a222140410 On latest Nightly, Aurora, Firefox 29 Beta 6 did not crash only hang. Ubuntu 13.10 32bit: Reproduced the crash on old nightly: bp-34bfe3f9-1753-4f50-b42f-8457d2140410 Latest Nightly: bp-ed458b5d-22d2-4484-9d84-3caec2140410 Latest Aurora: bp-55c2e25f-0d6e-4748-af24-14f712140410 Firefox 29 Beta 6: bp-1a8b9c9f-8434-4b30-b3b5-a210e2140410 Mac OS X 10.9.2 In 64bit mode: Reproduced a crash on old nightly: bp-42a0f235-341d-4d93-985c-c2c4a2140410 On latest Nightly, Aurora, Firefox 29 Beta 6 did not crash. In 32bit mode crash on Firefox 29 beta 6, latest Aurora and latest Nightly: bp-6e49465d-9561-44bc-b164-35c632140410 Windows 8.1 32bit Reproduced a crash on old nightly: bp-cd6007da-fb32-4795-be32-3e0902140410 Latest Nightly: bp-0cd301fb-c2f3-42f1-a868-f32b42140410 Latest Aurora: bp-c9425b20-cd8a-4be4-9e84-0d3fc2140410 Firefox 29 Beta 6: bp-df9617ab-4b2a-4a4d-a70e-2d1752140410 Windows 7 64bit Reproduced a crash on old nightly: bp-b3c4599e-aae9-4427-bc2a-19b872140410 Latest Nightly: bp-68cbe41f-74e2-48ba-b230-d52ca2140410 Latest Aurora: bp-8181a674-3f16-41f4-8971-30b332140410 Firefox 29 Beta 6:: bp-f9d6ff87-02e4-4482-9029-8273f2140410
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #13) > The crashes from Windows 8.1 32bit look the same for 'before' and 'after' > builds (check the crash reports bellow. Is this normal? Looks like the 32-bit Win 8.1 OS has only 2147352576 bytes of virtual memory and so this testcase runs out of memory before reaching the overflow. The aurora and beta 64-bit Win 7 crash reports are from 32-bit builds, which are the appropriate builds to test. These have 4294836224 bytes of virtual memory and are hitting the expected safe crashes.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.