Closed Bug 749860 Opened 13 years ago Closed 13 years ago

Heap-use-after-free in nsBorderColors

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla15
Tracking Flags:
Tracking Status
firefox14 --- fixed
firefox15 --- fixed
firefox-esr10 --- wontfix

People

(Reporter: inferno, Assigned: dbaron)

References

Details

(4 keywords, Whiteboard: [asan][advisory-tracking+])

Attachments

(2 files)

Testcase
154 bytes, text/html
Details
patch
1.29 KB, patch
bzbarsky
: review+
inferno
: feedback+
akeybl
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Attached file Testcase —
Checked Aurora, Trunk and could trigger it on both. ==26466== Warning: client program overrides the handler for signal 11. ==26466== Warning: client program overrides the handler for signal 11. ================================================================= ==26466== ERROR: AddressSanitizer heap-use-after-free on address 0x7f14b304d388 at pc 0x7f14e01b06ec bp 0x7fff9880e330 sp 0x7fff9880e328 READ of size 4 at 0x7f14b304d388 thread T0 #0 0x7f14e01b06ec in nsBorderColors firefox/aurora-src/layout/style/nsStyleStruct.h:633 #1 0x7f14e01b0408 in nsBorderColors firefox/aurora-src/layout/style/nsStyleStruct.h:633 #2 0x7f14e028f770 in nsBorderColors::Clone(bool) const firefox/aurora-src/layout/style/nsStyleStruct.cpp:443 #3 0x7f14e0158ab8 in nsBorderColors::Clone() const firefox/aurora-src/layout/style/nsStyleStruct.h:636 #4 0x7f14e0108649 in nsRuleNode::ComputeBorderData(void*, nsRuleData const*, nsStyleContext*, nsRuleNode*, nsRuleNode::RuleDetail, bool) firefox/aurora-src/layout/style/nsRuleNode.cpp:5608 #5 0x7f14e00b7017 in nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #6 0x7f14e017b36e in nsRuleNode::GetStyleBorder(nsStyleContext*, bool) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #7 0x7f14df1512f1 in nsStyleContext::DoGetStyleBorder(bool) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #8 0x7f14df0ca258 in nsStyleContext::GetStyleBorder() firefox/aurora-src/layout/style/nsStyleStructList.h:143 #9 0x7f14e0244e72 in nsStyleContext::CalcStyleDifference(nsStyleContext*) firefox/aurora-src/layout/style/nsStyleContext.cpp:478 #10 0x7f14df2dc0c5 in CaptureChange firefox/aurora-src/layout/base/nsFrameManager.cpp:1012 #11 0x7f14df2d4af3 in nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, nsRestyleHint, mozilla::css::RestyleTracker&, nsFrameManager::DesiredA11yNotifications, nsTArray<nsIContent*, nsTArrayDefaultAllocator>&, TreeMatchContext&) firefox/aurora-src/layout/base/nsFrameManager.cpp:1321 #12 0x7f14df2dde76 in nsFrameManager::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint, mozilla::css::RestyleTracker&, bool) firefox/aurora-src/layout/base/nsFrameManager.cpp:1696 #13 0x7f14df07147b in nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, bool) firefox/aurora-src/layout/base/nsCSSFrameConstructor.cpp:8031 #14 0x7f14defc0f3d in mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint) firefox/aurora-src/layout/base/RestyleTracker.cpp:158 #15 0x7f14defbb35d in mozilla::css::RestyleTracker::DoProcessRestyles() firefox/aurora-src/layout/base/RestyleTracker.cpp:243 #16 0x7f14df099baa in mozilla::css::RestyleTracker::ProcessRestyles() firefox/aurora-src/layout/base/RestyleTracker.h:103 #17 0x7f14df0996de in nsCSSFrameConstructor::ProcessPendingRestyles() firefox/aurora-src/layout/base/nsCSSFrameConstructor.cpp:11638 #18 0x7f14df44b416 in PresShell::FlushPendingNotifications(mozFlushType) firefox/aurora-src/layout/base/nsPresShell.cpp:3961 #19 0x7f14e0c7b3de in nsDocument::FlushPendingNotifications(mozFlushType) firefox/aurora-src/content/base/src/nsDocument.cpp:6343 #20 0x7f14e6249064 in nsDocLoader::DocLoaderIsEmpty(bool) firefox/aurora-src/uriloader/base/nsDocLoader.cpp:807 #21 0x7f14e624da7c in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/uriloader/base/nsDocLoader.cpp:736 #22 0x7f14e624f5ed in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/modules/zlib/src/inffast.c:0 #23 0x7f14ddbd0829 in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/netwerk/base/src/nsLoadGroup.cpp:731 #24 0x7f14e0c90c54 in nsDocument::DoUnblockOnload() firefox/aurora-src/content/base/src/nsDocument.cpp:7255 #25 0x7f14e0c906d1 in nsDocument::UnblockOnload(bool) firefox/aurora-src/content/base/src/nsDocument.cpp:7198 #26 0x7f14e0c42f34 in nsDocument::DispatchContentLoadedEvents() firefox/aurora-src/content/base/src/nsDocument.cpp:4269 #27 0x7f14e0cf3139 in nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() firefox/aurora-src/../../../dist/include/nsThreadUtils.h:345 #28 0x7f14e93f6f81 in nsThread::ProcessNextEvent(bool, bool*) firefox/aurora-src/xpcom/threads/nsThread.cpp:658 #29 0x7f14e9083bdd in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/aurora-src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:245 #30 0x7f14e85dc8c6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/aurora-src/ipc/glue/MessagePump.cpp:110 #31 0x7f14e96aee3a in MessageLoop::RunInternal() firefox/aurora-src/ipc/chromium/src/base/message_loop.cc:209 #32 0x7f14e96aec83 in MessageLoop::RunHandler() firefox/aurora-src/ipc/chromium/src/base/message_loop.cc:202 #33 0x7f14e96aeb68 in MessageLoop::Run() firefox/aurora-src/ipc/chromium/src/base/message_loop.cc:176 #34 0x7f14e7b167fe in nsBaseAppShell::Run() firefox/aurora-src/widget/xpwidgets/nsBaseAppShell.cpp:191 #35 0x7f14e66e6098 in nsAppStartup::Run() firefox/aurora-src/toolkit/components/startup/nsAppStartup.cpp:295 #36 0x7f14dd998323 in XRE_main firefox/aurora-src/toolkit/xre/nsAppRunner.cpp:3703 #37 0x40a1f3 in do_main firefox/aurora-src/browser/app/nsBrowserApp.cpp:190 #38 0x407d7e in main firefox/aurora-src/browser/app/nsBrowserApp.cpp:277 #39 0x7f14f5c31c4d in ?? ??:0 0x7f14b304d388 is located 8 bytes inside of 16-byte region [0x7f14b304d380,0x7f14b304d390) freed by thread T0 here: #0 0x42b972 in free ??:0 #1 0x7f14f419a673 in moz_free firefox/aurora-src/memory/mozalloc/mozalloc.cpp:98 #2 0x7f14e015833d in nsStyleBorder::ClearBorderColors(mozilla::css::Side) firefox/aurora-src/layout/style/nsStyleStruct.h:780 #3 0x7f14e0108617 in nsRuleNode::ComputeBorderData(void*, nsRuleData const*, nsStyleContext*, nsRuleNode*, nsRuleNode::RuleDetail, bool) firefox/aurora-src/layout/style/nsRuleNode.cpp:5608 #4 0x7f14e00b7017 in nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #5 0x7f14e017b36e in nsRuleNode::GetStyleBorder(nsStyleContext*, bool) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #6 0x7f14df1512f1 in nsStyleContext::DoGetStyleBorder(bool) firefox/aurora-src/layout/xul/base/src/tree/src/../../../../../style/nsStyleStructList.h:143 #7 0x7f14df0ca258 in nsStyleContext::GetStyleBorder() firefox/aurora-src/layout/xul/base/src/tree/src/../../../../../style/nsStyleStructList.h:143 #8 0x7f14e0244e72 in nsStyleContext::CalcStyleDifference(nsStyleContext*) firefox/aurora-src/layout/style/nsStyleContext.cpp:478 #9 0x7f14df2dc0c5 in CaptureChange firefox/aurora-src/layout/base/nsFrameManager.cpp:1012 #10 0x7f14df2d4af3 in nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, nsRestyleHint, mozilla::css::RestyleTracker&, nsFrameManager::DesiredA11yNotifications, nsTArray<nsIContent*, nsTArrayDefaultAllocator>&, TreeMatchContext&) firefox/aurora-src/layout/base/nsFrameManager.cpp:1321 #11 0x7f14df2dde76 in nsFrameManager::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint, mozilla::css::RestyleTracker&, bool) firefox/aurora-src/layout/base/nsFrameManager.cpp:1696 #12 0x7f14df07147b in nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, bool) firefox/aurora-src/layout/base/nsCSSFrameConstructor.cpp:8031 #13 0x7f14defc0f3d in mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint) firefox/aurora-src/layout/base/RestyleTracker.cpp:158 #14 0x7f14defbb35d in mozilla::css::RestyleTracker::DoProcessRestyles() firefox/aurora-src/layout/base/RestyleTracker.cpp:243 #15 0x7f14df099baa in mozilla::css::RestyleTracker::ProcessRestyles() firefox/aurora-src/layout/base/RestyleTracker.h:103 #16 0x7f14df0996de in nsCSSFrameConstructor::ProcessPendingRestyles() firefox/aurora-src/layout/base/nsCSSFrameConstructor.cpp:11638 #17 0x7f14df44b416 in PresShell::FlushPendingNotifications(mozFlushType) firefox/aurora-src/layout/base/nsPresShell.cpp:3961 #18 0x7f14e0c7b3de in nsDocument::FlushPendingNotifications(mozFlushType) firefox/aurora-src/content/base/src/nsDocument.cpp:6343 #19 0x7f14e6249064 in nsDocLoader::DocLoaderIsEmpty(bool) firefox/aurora-src/uriloader/base/nsDocLoader.cpp:807 #20 0x7f14e624da7c in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/uriloader/base/nsDocLoader.cpp:736 #21 0x7f14e624f5ed in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/modules/zlib/src/inffast.c:0 #22 0x7f14ddbd0829 in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/netwerk/base/src/nsLoadGroup.cpp:731 #23 0x7f14e0c90c54 in nsDocument::DoUnblockOnload() firefox/aurora-src/content/base/src/nsDocument.cpp:7255 #24 0x7f14e0c906d1 in nsDocument::UnblockOnload(bool) firefox/aurora-src/content/base/src/nsDocument.cpp:7198 #25 0x7f14e0c42f34 in nsDocument::DispatchContentLoadedEvents() firefox/aurora-src/content/base/src/nsDocument.cpp:4269 #26 0x7f14e0cf3139 in nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() firefox/aurora-src/../../../dist/include/nsThreadUtils.h:345 #27 0x7f14e93f6f81 in nsThread::ProcessNextEvent(bool, bool*) firefox/aurora-src/xpcom/threads/nsThread.cpp:658 #28 0x7f14e9083bdd in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/aurora-src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:245 #29 0x7f14e85dc8c6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/aurora-src/ipc/glue/MessagePump.cpp:110 previously allocated by thread T0 here: #0 0x42ba32 in malloc ??:0 #1 0x7f14f419a7c7 in moz_xmalloc firefox/aurora-src/memory/mozalloc/mozalloc.cpp:103 #2 0x7f14e028f74a in nsBorderColors::Clone(bool) const firefox/aurora-src/layout/style/nsStyleStruct.cpp:443 #3 0x7f14e0158ab8 in nsBorderColors::Clone() const firefox/aurora-src/layout/style/nsStyleStruct.h:636 #4 0x7f14e0290d35 in nsStyleBorder firefox/aurora-src/layout/style/nsStyleStruct.cpp:475 #5 0x7f14e0106485 in nsRuleNode::ComputeBorderData(void*, nsRuleData const*, nsStyleContext*, nsRuleNode*, nsRuleNode::RuleDetail, bool) firefox/aurora-src/layout/style/nsRuleNode.cpp:5450 #6 0x7f14e00b7017 in nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #7 0x7f14e017b36e in nsRuleNode::GetStyleBorder(nsStyleContext*, bool) firefox/aurora-src/layout/style/nsStyleStructList.h:143 #8 0x7f14df1512f1 in nsStyleContext::DoGetStyleBorder(bool) firefox/aurora-src/layout/xul/base/src/tree/src/../../../../../style/nsStyleStructList.h:143 #9 0x7f14df0ca258 in nsStyleContext::GetStyleBorder() firefox/aurora-src/layout/xul/base/src/tree/src/../../../../../style/nsStyleStructList.h:143 #10 0x7f14e0244e72 in nsStyleContext::CalcStyleDifference(nsStyleContext*) firefox/aurora-src/layout/style/nsStyleContext.cpp:478 #11 0x7f14df2dc0c5 in CaptureChange firefox/aurora-src/layout/base/nsFrameManager.cpp:1012 #12 0x7f14df2d4af3 in nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint, nsRestyleHint, mozilla::css::RestyleTracker&, nsFrameManager::DesiredA11yNotifications, nsTArray<nsIContent*, nsTArrayDefaultAllocator>&, TreeMatchContext&) firefox/aurora-src/layout/base/nsFrameManager.cpp:1321 #13 0x7f14df2dde76 in nsFrameManager::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint, mozilla::css::RestyleTracker&, bool) firefox/aurora-src/layout/base/nsFrameManager.cpp:1696 #14 0x7f14df07147b in nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, bool) firefox/aurora-src/layout/base/nsCSSFrameConstructor.cpp:8031 #15 0x7f14defc0f3d in mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint) firefox/aurora-src/layout/base/RestyleTracker.cpp:158 #16 0x7f14defbb35d in mozilla::css::RestyleTracker::DoProcessRestyles() firefox/aurora-src/layout/base/RestyleTracker.cpp:243 #17 0x7f14df099baa in mozilla::css::RestyleTracker::ProcessRestyles() firefox/aurora-src/layout/base/RestyleTracker.h:103 #18 0x7f14df0996de in nsCSSFrameConstructor::ProcessPendingRestyles() firefox/aurora-src/layout/base/nsCSSFrameConstructor.cpp:11638 #19 0x7f14df44b416 in PresShell::FlushPendingNotifications(mozFlushType) firefox/aurora-src/layout/base/nsPresShell.cpp:3961 #20 0x7f14e0c7b3de in nsDocument::FlushPendingNotifications(mozFlushType) firefox/aurora-src/content/base/src/nsDocument.cpp:6343 #21 0x7f14e6249064 in nsDocLoader::DocLoaderIsEmpty(bool) firefox/aurora-src/uriloader/base/nsDocLoader.cpp:807 #22 0x7f14e624da7c in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/aurora-src/uriloader/base/nsDocLoader.cpp:736 ==26466== ABORTING Stats: 152M malloced (152M for red zones) by 328426 calls Stats: 40M realloced by 18121 calls Stats: 131M freed by 224886 calls Stats: 0M really freed by 0 calls Stats: 336M (86065 full pages) mmaped in 84 calls mmaps by size class: 8:262128; 9:49146; 10:20475; 11:16376; 12:3072; 13:2048; 14:1536; 15:384; 16:640; 17:192; 18:112; 19:48; 20:16; mallocs by size class: 8:243052; 9:47441; 10:17171; 11:14146; 12:2135; 13:1762; 14:1418; 15:342; 16:615; 17:185; 18:98; 19:48; 20:13; frees by size class: 8:153448; 9:39298; 10:14983; 11:11624; 12:1556; 13:1563; 14:1247; 15:304; 16:551; 17:171; 18:87; 19:44; 20:10; rfrees by size class: Stats: malloc large: 344 small slow: 1806 Shadow byte and word: 0x1fe296609a71: fd 0x1fe296609a70: fd fd fd fd fd fd fd fd More shadow bytes: 0x1fe296609a50: 00 00 00 00 fb fb fb fb 0x1fe296609a58: fb fb fb fb fb fb fb fb 0x1fe296609a60: fa fa fa fa fa fa fa fa 0x1fe296609a68: fa fa fa fa fa fa fa fa =>0x1fe296609a70: fd fd fd fd fd fd fd fd 0x1fe296609a78: fd fd fd fd fd fd fd fd 0x1fe296609a80: fa fa fa fa fa fa fa fa 0x1fe296609a88: fa fa fa fa fa fa fa fa 0x1fe296609a90: 00 00 fb fb fb fb fb fb
Component: Security → Style System (CSS)
Product: Firefox → Core
QA Contact: firefox → style-system
I am on http://hg.mozilla.org/mozilla-central/rev/0f8ea3826bf7 (Fri Apr 27 08:39:28 2012) ASANified Trunk and http://hg.mozilla.org/releases/mozilla-aurora/rev/2949b3533041 (Fri Apr 27 09:13:39 2012) ASANified Aurora.
Yeah, this looks like a bug in nsRuleNode::ComputeBorderData. The code there is assuming that parentBorder != border, which isn't necessarily the case. (In particular, when there's an 'inherit' value, we ensure that parentBorder is a sensible value for anything other than the root; however, we're dealing with the root here, so parentBorder == border.) (See COMPUTE_START_RESET.)
In particular, the broken code is here: case eCSSUnit_Inherit: { canStoreInRuleTree = false; nsBorderColors *parentColors; parentBorder->GetCompositeColors(side, &parentColors); if (parentColors) { border->EnsureBorderColors(); border->ClearBorderColors(side); border->mBorderColors[side] = parentColors->Clone(); } else { border->ClearBorderColors(side); } break; }
Attached patch patch — — Splinter Review
I haven't tested this, but I think it's the problem.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #619249 - Flags: review?(bzbarsky)
Comment on attachment 619249 [details] [diff] [review] patch Would you mind testing that this fixes the problem
Attachment #619249 - Flags: feedback?(inferno)
Flags: in-testsuite?
Comment on attachment 619249 [details] [diff] [review] patch r=me
Attachment #619249 - Flags: review?(bzbarsky) → review+
Comment on attachment 619249 [details] [diff] [review] patch Tested the patch on trunk. It fixes the use-after-free.
Attachment #619249 - Flags: feedback?(inferno) → feedback+
http://hg.mozilla.org/mozilla-central/rev/1d3c3f280f60
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
status-firefox15: --- → fixed
Resolution: --- → FIXED
Whiteboard: [asan]
Comment on attachment 619249 [details] [diff] [review] patch [Approval Request Comment] Regression caused by (bug #): bug 389404 (December 2008) User impact if declined: crash, probably denial of service (I *think* it's not exploitable, but I'm not 100% sure about that) Testing completed (on m-c, etc.): on mozilla-central, patch tested to fix bug Risk to taking this patch (and alternatives if risky): low; affects relatively obscure CSS property used pretty much only by our own chrome (if that anymore) String changes made by this patch: none
Attachment #619249 - Flags: approval-mozilla-aurora?
Comment on attachment 619249 [details] [diff] [review] patch [Triage Comment] Low risk sg:dos fix. Given where we are in the cycle, approving for Aurora 14.
Attachment #619249 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Since we believe this is just an sg:dos, we've opted to wontfix for ESR10.
status-firefox-esr10: --- → wontfix
Verified fixed in my 5/10 ASAN build.
Status: RESOLVED → VERIFIED
Is this really just a DOS security rating? We didn't mark the keywords. Dan Veditz?
Whiteboard: [asan] → [asan][advisory-tracking+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: