Closed Bug 1243623 Opened 9 years ago Closed 9 years ago

Use-after-free (poisoned) in nsTableFrame::FixupPositionedTableParts

Categories

(Core :: Layout, defect)

44 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
firefox44 --- affected
firefox45 --- affected
firefox47 --- verified
firefox-esr38 --- unaffected

People

(Reporter: amatcama, Assigned: roc)

References

Details

(6 keywords, Whiteboard: [sg:dos][adv-main47+])

Attachments

(4 files)

Attached file poc (deleted) —
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
Attached file backtrace + instruction + registers (deleted) —
[Tracking Requested - why for this release]: UAF
Group: firefox-core-security → core-security
Component: Untriaged → Layout
Product: Firefox → Core
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.
Group: core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [sg:dos]
Blocks: 1209994
Make sure Roc is aware of this. Prioritize as appropriate.
Flags: needinfo?(roc)
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)
Attached file crash.html (deleted) —
Improved testcase
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?
Group: layout-core-security
Flags: needinfo?(jfkthame)
(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.
Flags: needinfo?(jfkthame)
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.
Attachment #8717671 - Flags: sec-approval?
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.
Thanks.
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.
Attachment #8717671 - Flags: sec-approval?
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Group: layout-core-security → core-security-release
Whiteboard: [sg:dos] → [sg:dos][adv-main47+]
Alias: CVE-2016-2823
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.
Status: RESOLVED → VERIFIED
Alias: CVE-2016-2823
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: