Closed Bug 1095510 Opened 5 years ago Closed 5 years ago

crash in sse2_fill (EXCEPTION_ACCESS_VIOLATION_WRITE)

Categories

(Core :: Graphics, defect, critical)

x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED INCOMPLETE
Tracking Status
firefox33 --- wontfix
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- wontfix
firefox40 --- wontfix
firefox-esr31 --- wontfix

People

(Reporter: davidb, Assigned: milan)

References

Details

(Keywords: crash, sec-critical, Whiteboard: mostly WinXP-only)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-5959553a-84ad-4a8d-b1da-459652141106.
=============================================================

That was a 35.0a2 stack. Here's a 33.0.3 stack:
https://crash-stats.mozilla.com/report/index/ecb844e4-b362-4d6b-80bc-4f2322141107

More here:
https://crash-stats.mozilla.com/report/list?signature=sse2_fill#tab-reports

Automated exploitability dump analysis rates the majority of these crashes as 'high' (which generally indicates high or critical).

(Memories of bug 825721?)
Jeff: this appears to be an exploitable crash that's common on WinXP (96%) and uncommon elsewhere. Anything jump out at you? The actual crash location appears to be code untouched since a 2010 pixman update so I don't know if something more recent has caused this crash to be more common. Some old versions do experience the crash:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=sse2_fill#tab-sigsummary
Group: gfx-core-security
Flags: needinfo?(jmuizelaar)
Keywords: sec-critical
Whiteboard: mostly WinXP-only
This is not likely a pixman crash. The upper layers of the code are probably writing/reading bad data to an invalid buffer. This is common on WinXP because that's where we use software rendering the most. There are not really good ways to proceed on this without being able to reproduce it.
Flags: needinfo?(jmuizelaar)
If dealing with this is high priority we could probably add some kind of canary to the surface that would cause us to crash earlier.
Group: gfx-core-security
Milan, we are treating this as sec-critical. Can you perhaps find someone who might be able to dig deeper on this, based on comment 3?
Flags: needinfo?(milan)
Some of these crashes are on nightly (e.g., https://crash-stats.mozilla.com/report/index/75422f07-d8e2-4a59-8eed-6ebed2141205) and in DrawTargetCairo::ClearRect, so that's probably a good place to put a gfxCriticalError() so that we get information about the rectangle we're requesting to clear in the crash report.
Let me run a patch on try, see if we can land something ugly for a couple of days, just to collect the data.
Attached patch Aurora patch (obsolete) — Splinter Review
A shot in the dark, not sure if there is a better place to start.  This would at least give us the size/location of the last rectangle in ClearRect before the crash, if the size or location was infinite or negative.  That should still be OK, but at least it may tell us if there is a correlation on the nightly.  Is it worth getting this on nightly for a few days?  It won't do anything about the crashes themselves, the would still happen the same way.
Flags: needinfo?(milan)
Attachment #8535095 - Flags: review?(jmuizelaar)
Does this affect ESR31?
Comment on attachment 8535095 [details] [diff] [review]
Aurora patch

Review of attachment 8535095 [details] [diff] [review]:
-----------------------------------------------------------------

Sure.
Attachment #8535095 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8535095 [details] [diff] [review]
Aurora patch

This is actually not a fix, we're just trying to add some more information to crash reports to see if we can get the right vector for the solution.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
It should be fairly easy to trigger the warning that was added in this patch.  It is unclear that would always lead to a crash though and trigger this bug.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Which older supported branches are affected by this flaw?
All

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?

How likely is this patch to cause regressions; how much testing does it need?
Attachment #8535095 - Flags: sec-approval?
Attachment #8535095 - Flags: sec-approval? → sec-approval+
Assignee: nobody → milan
https://hg.mozilla.org/integration/mozilla-inbound/rev/8942a3b92db3

Are we going to want to land the original patch for the branch uplifts since bug 1101685 isn't present?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8942a3b92db3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
I'm not yet seeing crashes on nightly with a new enough build.

(In reply to [Away 24-Dec to 4-Jan] Ryan VanderMeulen [:RyanVM UTC-5] from comment #15)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8942a3b92db3
> 
> Are we going to want to land the original patch for the branch uplifts since
> bug 1101685 isn't present?

There are crashes with "new enough" Aurora - let's uplift the original patch to Aurora, see if we get some more information from that population.
Comment on attachment 8535095 [details] [diff] [review]
Aurora patch

Approval Request Comment

We're not getting information from the nightly, but there seems to be more of them on Aurora, with "fresh" builds.
Attachment #8535095 - Attachment description: Worth putting this in? → Aurora patch
Attachment #8535095 - Flags: approval-mozilla-aurora?
Attachment #8535095 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/db860c3d4f45

Fx35 goes to RC today, so that's kinda crappy for a sec-crit. I guess this is wontfix for Fx35 and needed ASAP for esr31?
Flags: needinfo?(milan)
wontfix for 35 makes sense; esr31, as soon as we actually get some data, since this is not actually a fix.
Flags: needinfo?(milan)
In that case, reopening the bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla37 → ---
At least one ClearRect that crashes (https://crash-stats.mozilla.com/report/index/af3e84ab-c25d-4e08-aba5-175402150108) without triggering this particular canary.
No crashes on nightly since 38 started, but that could be just the sampling...
Have there been any crashes with useful information in the last few weeks?
ni? for comment 24.  Is there something to do here?
Flags: needinfo?(milan)
There is on recent crash report 9ce7330b-1b62-4e8a-b0f0-c1fca2150303 and it does not hit the additional code, so it's back to the drawing board.
Flags: needinfo?(milan)
Here's something far fetched; bug 1125925 has UAF with cairo clips; although we don't crash in that code here, we do it right after we were interacting with cairo clips.  Conspiracy theory.
Depends on: 1125925
This bug is four months old, and we don't have any real information about what is happening, so I'm going to close it incomplete.  Hopefully bug 1125925 will get fixed at some point and help here.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → INCOMPLETE
Andrew flagged this as a possible duplicate of bug 1166082.
Group: core-security → core-security-release
Group: gfx-core-security, core-security-release
You need to log in before you can comment on or make changes to this bug.