Closed Bug 1243623 Opened 5 years ago Closed 5 years ago
Use-after-free (poisoned) in ns
Table Frame::Fixup Positioned Table Parts
1.14 KB, text/html
13.99 KB, text/plain
348 bytes, text/html
5.86 KB, patch
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.82 Safari/537.36 Steps to reproduce: firefox uaf.html Actual results: firefox crashed Expected results: firefox shouldn't have crashed
[Tracking Requested - why for this release]: UAF
Tracking for all current versions since this looks like a bad crash.
In a Firefox 44 release build I get a null deref: bp-7fb09c44-0944-42b1-97e0-c473c2160130 As in the attached stack trace I see register values with 0x7ffffffff0deaxxx which is our frame-poisoned address. It's a UAF of a nsIFrame object which we've specifically neutered by pointing to unmapped memory to cause guaranteed crashes. We can treat this as a DOS crash. I do NOT crash in ESR 38.6 so this problem was introduced somewhere between 38 and 44.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Make sure Roc is aware of this. Prioritize as appropriate.
I do not crash in f53533d9eb77 (Feb 4 Nightly). Alice, does this crash in Nightly for you? If not, can you get a fix range? Thanks!!!
Flags: needinfo?(roc) → needinfo?(alice0775)
Fixed range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=94c27b21c934f0cc178e896587532d7cc0b4c60c&tochange=6f4756d4bab615ade4e575b2f3343772a926abd0
Assignee: nobody → roc
The problem is that we're creating an nsTableFrame and splitting it in a non-print context. Our table code, in particular our positioned-table-part handling, is crashing on dynamic changes to tables with continuations. We can probably fix the particular crash here but dynamic changes to tables with continuations is generally known to fail. The patches in bug 1243125 fix the root cause that's allowing non-printed tables to split. We should uplift them everywhere; although bug 1209994 exposed this particular crash, there are almost certainly others that work before bug 1209994 was fixed. Jonathan, can you backport the bug 1243125 fixes?
5 years ago
Attachment #8717671 - Flags: review?(mats)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10) > The patches in bug 1243125 fix the root cause that's allowing non-printed > tables to split. We should uplift them everywhere; although bug 1209994 > exposed this particular crash, there are almost certainly others that work > before bug 1209994 was fixed. Jonathan, can you backport the bug 1243125 > fixes? It looks like they'll uplift easily; I've posted a rollup patch there and requested approval.
Comment on attachment 8717671 [details] [diff] [review] Don't skip unregistering a table part if we have a split table >+ // The table frame will be destroyed, and it's the first im flow (and thus >+ // owning the PositionedTablePartArray), so we don't need to do >+ // anything. s/im/in/ and it looks like "anything." fits on the second line?
Attachment #8717671 - Flags: review?(mats) → review+
Comment on attachment 8717671 [details] [diff] [review] Don't skip unregistering a table part if we have a split table Review of attachment 8717671 [details] [diff] [review]: ----------------------------------------------------------------- Not sure when I should land this ... we believe this particular bug is blocked by frame poisoning, but the general issue of tables being split might lead to other exploitable bugs.
One question I had about frame poisoning, are the frames ever "recycled" ? Suppose I create a large number of the same objects which end up in the same frame. What happens if I free all these objects and the frame is "empty" ? Is this frame ever released and then later used as a frame for other objects ? Because I don't understand how memory leaks could be prevented if this is not the case. Thanks
> are the frames ever "recycled" ? The "frame" in "frame poisoning" refers to the CSS box for a DOM node (roughly). I.e. subclasses of nsIFrame: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsIFrame.h#415 These are allocated from an memory arena per document and not free'd in the normal malloc/free sense. Instead they are cached, and re-used for the next element that needs a frame *of that type*. So once a memory location has been allocated for say a table frame it will always be a table frame at that memory location. Also, while the frame is unused in the cache it's filled with "poison" which will lead to a crash if you try to use it as a pointer. Both these properties are good for security, either you crash, or if a new frame is re-using the memory, it's of the same type as the last one (which prevents using the vtable of a table frame with a frame of a different type for example). After the document is destroyed though the whole memory arena is free'd so the memory can be reused for arbitrary things. A use-after-free of a frame after that happens can be exploitable. That's very rare though. You're right that there's sort of a memory leak here, because we will never go under the high-water mark for the number of frames that was allocated in a document. For typical web content this spill should be fairly low though. Frame poisoning has mitigated a ton of bugs so it's a small price to pay.
Comment on attachment 8717671 [details] [diff] [review] Don't skip unregistering a table part if we have a split table Making this a sec-low after talking to Dan Veditz. Doesn't need sec-approval to go in unless it is sec-high or sec-critical so clearing that. Check in.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ba4a380e9020f75cf016ca358c3982031385434 Bug 1243623. Don't skip unregistering a table part if we have a split table. r=mats
Whiteboard: [sg:dos] → [sg:dos][adv-main47+]
I've managed to reproduce the crash using poc.html/crash.html on an affected nightly 47.0a1 build(20160204030229) from 2016-02-04, under Windows 10 x64. This is no longer reproducible with 47.0b9 build (20160526140250) on the following platforms: Windows 10 x64, Mac OS X 10.9.5 and Ubuntu 14.04 LTS x64.
You need to log in before you can comment on or make changes to this bug.