13.63 KB, text/plain
8.24 KB, text/plain
10.70 KB, text/plain
1.78 KB, patch
|Details | Diff | Splinter Review|
Created attachment 8694807 [details] AddressSanitizer: heap-buffer-overflow report 1. http://autojavier.cochesdeocasion.com/ 2. AddressSanitizer: heap-buffer-overflow in mozilla::image::Deinterlacer::PropagatePassToDownscaler Beta/43, Aurora/44, Nightly/45 at least.
This looks like it could be a regression from bug 1194058. Can you look into this, Seth? Thanks.
confirmed on windows 7 as assertion failure
Group: core-security → gfx-core-security
(In reply to Andrew McCreight [:mccr8] from comment #2) > This looks like it could be a regression from bug 1194058. Can you look into > this, Seth? Thanks. mozregression agrees. Like bug 1231121, it sometimes required a couple force-refreshes to reliably reproduce for me. Also confirmed that this doesn't reproduce on Fx42 or ESR38.5. FWIW, here's an Fx43 crash reporter link: https://crash-stats.mozilla.com/report/index/def60afa-2c9b-4e22-90cd-289852151215
status-b2g-v2.0: --- → unaffected
status-b2g-v2.0M: --- → unaffected
status-b2g-v2.1: --- → unaffected
status-b2g-v2.1S: --- → unaffected
status-b2g-v2.2: --- → unaffected
status-b2g-v2.2r: --- → unaffected
status-b2g-v2.5: --- → affected
status-b2g-master: --- → affected
status-firefox42: --- → unaffected
status-firefox43: --- → affected
status-firefox44: --- → affected
status-firefox45: --- → affected
status-firefox46: --- → affected
status-firefox-esr38: --- → unaffected
status-firefox-esr45: --- → affected
tracking-firefox43: --- → ?
tracking-firefox44: --- → ?
tracking-firefox45: --- → ?
tracking-firefox46: --- → ?
tracking-firefox-esr45: --- → ?
Tracked for FF44 since it's a sec-high.
tracking-firefox44: ? → +
Looks like I missed a case in bug 1233465. Patch up soon.
Assignee: nobody → edwin
Status: NEW → ASSIGNED
2 years ago
Duplicate of this bug: 1233651
Bug 1233651 was marked sec-critical. Should we be reconsidering the rating in this bug? Also, would be great if we could eventually land a test for this bug.
Created attachment 8701081 [details] [diff] [review] 1233651-crash.patch sigh.
Attachment #8701081 - Flags: review?(seth)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9) > Bug 1233651 was marked sec-critical. Should we be reconsidering the rating > in this bug? Sure. Looks like this is a write, too.
Keywords: sec-high → sec-critical
Too late for 43. Sorry. Just a reminder, looks like this should have sec approval before it lands.
status-firefox43: affected → wontfix
tracking-firefox43: ? → +
tracking-firefox46: ? → +
This is a critical security issue that has existed since September and is a common crash in reproduced in Bughunter which is being experienced by our users. What will it take to get this 2 line patch reviewed and checked in?
Attachment #8701081 - Flags: review?(seth) → review+
Comment on attachment 8701081 [details] [diff] [review] 1233651-crash.patch [Security approval request comment] How easily could an exploit be constructed based on the patch? It would be reasonably easy to construct a crashing GIF, or write to immediately adjacent memory on the heap. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Perhaps we should take the word "bounds" out of the check-in comment... Which older supported branches are affected by this flaw? 43+ If not all supported branches, which bug introduced the flaw? bug 1194058 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Backporting should be trivial. How likely is this patch to cause regressions; how much testing does it need? It couldn't possibly make things worse. Worst case is it fails to make things better, but it certainly won't make them worse.
Attachment #8701081 - Flags: sec-approval?
Comment on attachment 8701081 [details] [diff] [review] 1233651-crash.patch sec-approval+ for trunk. Edwin, can you make and nominate patches for Aurora and Beta?
Attachment #8701081 - Flags: sec-approval? → sec-approval+
tracking-firefox45: ? → +
tracking-firefox-esr45: ? → ---
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-b2g-master: affected → fixed
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment on attachment 8701081 [details] [diff] [review] 1233651-crash.patch Applies cleanly on Aurora and Beta. Approval Request Comment [Feature/regressing bug #]: bug 1194058 [User impact if declined]: crashing on malformed gifs [Describe test coverage new/current, TreeHerder]: Fixes the crash for me locally. [Risks and why]: Very little; this patch just makes some bounds smaller. [String/UUID change made/needed]: None.
Comment on attachment 8701081 [details] [diff] [review] 1233651-crash.patch Taking it. Should be in 44 beta 6.
status-firefox44: affected → fixed
status-firefox45: affected → fixed
Group: gfx-core-security → core-security-release
This broke builds without skia because of the lack of a FrameSize method in the Downscaler class: http://hg.mozilla.org/releases/mozilla-beta/file/tip/image/Downscaler.h#l150 FrameSize is now in central in that file, added in bug 1235859. Edwin, would you mind doing an approval request for aurora/beta on that bug?
Reproduced the crash using Firefox 43.0.4 and Nightly 45.0a1 2015-12-05 under Win 7 (x64). The crash no longer occurs on Firefox 44 beta 8, latest Aurora 45.0a2 and latest Nightly 46.0a1 2016-01-13 under Win 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5. Still, there are a few crash reports in Socorro, after the fix landed, such as: 44 beta 7: https://crash-stats.mozilla.com/report/index/0946d7e3-a9a6-46b2-8946-d233f2160110 44 beta 8: https://crash-stats.mozilla.com/report/index/0f774792-734d-41eb-8002-7f0802160113 46.0a1 2016-01-11: https://crash-stats.mozilla.com/report/index/0830dfa8-38b1-473e-9406-051ea2160112 45.0a2 2016-01-12: https://crash-stats.mozilla.com/report/index/57c5ead7-d9e1-495a-896f-8e4312160113 Edwin, could you please see if those are related? Thank you
(In reply to Petruta Rasa [QA] [:petruta] from comment #23) > Still, there are a few crash reports in Socorro, after the fix landed, such > as: > 44 beta 7: > https://crash-stats.mozilla.com/report/index/0946d7e3-a9a6-46b2-8946- > d233f2160110 > 44 beta 8: > https://crash-stats.mozilla.com/report/index/0f774792-734d-41eb-8002- > 7f0802160113 > 46.0a1 2016-01-11: > https://crash-stats.mozilla.com/report/index/0830dfa8-38b1-473e-9406- > 051ea2160112 > 45.0a2 2016-01-12: > https://crash-stats.mozilla.com/report/index/57c5ead7-d9e1-495a-896f- > 8e4312160113 > > Edwin, could you please see if those are related? Thank you Why are those crash reports thought to be related to this bug? They have a completely different stack in a different area of gecko.
(In reply to Timothy Nikkel (:tnikkel) from comment #24) > Why are those crash reports thought to be related to this bug? They have a > completely different stack in a different area of gecko. They all appear for the same signature in crash-stats: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=nsTextFrame%3A%3AClearTextRun#tab-reports
I've got the same signature https://crash-stats.mozilla.com/report/index/bp-cc85f4a8-09fc-422a-9e3d-3b69a2160114 while hard refreshing the http://autojavier.cochesdeocasion.com/ website with 43.0.4. If those are not related, how could I check this bug is fixed? Thank you!
(In reply to Petruta Rasa [QA] [:petruta] from comment #27) > I've got the same signature > https://crash-stats.mozilla.com/report/index/bp-cc85f4a8-09fc-422a-9e3d- > 3b69a2160114 while hard refreshing the > http://autojavier.cochesdeocasion.com/ website with 43.0.4. > > If those are not related, how could I check this bug is fixed? Thank you! Hmm, that crash is either a different issue, or it's getting the wrong stack somehow, perhaps it's showing the main thread stack (which isn't crashing) at the time that a decoder thread crashes? In any case if you crashed before, and no longer crash you can call this bug verified. If there are still crashes with that signature they are likely different issues.
As Timothy says, those are different so a new bug should be filed. Clearing ni?.
status-firefox-esr45: affected → fixed
Thanks! Reproduced the crash also on Linux x64 asan build from 2015-12-02. It no longer occurs on newer asan builds. Marking as verified fixed based on this and the above comments.
status-firefox44: fixed → verified
status-firefox45: fixed → verified
status-firefox46: fixed → verified
You need to log in before you can comment on or make changes to this bug.