Closed Bug 1138092 Opened 5 years ago Closed 5 years ago
Crash at xul!Is
Ruby Align Space Around
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.115 Safari/537.36 Steps to reproduce: 1. Upload the attached sample to a local webserver. 2. Open Firefox Nightly and navigate to the sample file. Actual results: Crash at xul!IsRubyAlignSpaceAround Expected results: No crash. Notes: - It seems a null ptr dereference but marked as security until it's confirmed by an expert. - Tested on: * Firefox Nightly Builds 39.0a1 (2015-02-28). It does not repro in Firefox 36. * WindowsXP, Win7 and Win8.1.
I do see one report of the crash @IsRubyAlignSpaceAround in crash stats from a Feb 28 build bp-ac47ec95-d18b-497e-aaf5-1578c2150228 Today I crash with a similar stack in the function above, @nsLineLayout::TextAlignLine bp-174ba9d2-8271-4b2d-8cf8-b28122150302 Both appear to be null derefs, but I can't tell how seriously we're confused to get to that point and whether a more useful (to an attacker) crash could be produced. The style tags don't appear to be necessary to the testcase. This works: data:text/html,<html><rt><canvas><fieldset></fieldset><rb></html> Jesse: are ruby-related tags being picked up in your fuzzer? If not is there a problem with the fuzzer or is it a lack of source material in the testsuites? It seems to be a big chunk of new layout code which is always scary.
Status: UNCONFIRMED → NEW
Crash Signature: [@ IsRubyAlignSpaceAround ] [@ nsLineLayout::TextAlignLine(nsLineBox*, bool) ]
Component: Untriaged → Layout
Ever confirmed: true
Product: Firefox → Core
This crash is a regression of bug 1055676, but the intrinsic problem is something broken from the beginning.
I have no idea how to fix this problem. The problem here is that, some HTML elements may generate an anonymous box to wrap its children, even if it has an inline display, especially form controls. This breaks the concept of IsInlineDescendantOfRuby as well as the style computation depending on it. It means we need to know the frame construction data before we can compute it. heycam suggested me that I can add a flag during constructing frames to notify the style context. But the problem is that style context could be created very early. During dynamic changes, children of a context style may even be created earlier than the frame construction item. For this very bug, though, the crash can be fixed by adding some sanity check to the conditions. But the true problem doesn't seem to have an easy solution. dbaron, any idea?
I honestly wouldn't worry too much about being correct here; the ruby spec is doing pretty crazy things, architecturally, that other layout specs simply haven't done, and I don't think we should worry that much about edge cases like form controls inside of ruby. It just shouldn't crash.
Assignee: nobody → quanxunzhen
Attachment #8572219 - Flags: review?(dbaron)
Attachment #8572219 - Flags: review?(dbaron) → review+
Comment on attachment 8572219 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: bug 1055676 [User impact if declined]: crash for some content [Describe test coverage new/current, TreeHerder]: will add crashtest [Risks and why]: no risk, only add a sanity check to avoid crash [String/UUID change made/needed]: n/a
Attachment #8572219 - Flags: approval-mozilla-aurora?
Attachment #8572219 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.