Closed
Bug 804927
(CVE-2012-5839)
Opened 12 years ago
Closed 12 years ago
heap-buffer-overflow in gfxShapedWord::CompressedGlyph::IsClusterStart
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: inferno, Assigned: jfkthame)
Details
(5 keywords, Whiteboard: [asan][adv-track-main17+][adv-track-esr17+])
Attachments
(3 files, 3 obsolete files)
|
910 bytes,
text/html
|
Details | |
|
1.99 KB,
patch
|
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
|
2.29 KB,
patch
|
Details | Diff | Splinter Review |
Reproduces on trunk.
=================================================================
==18316== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7f564317a4c8 at pc 0x7f56663d9456 bp 0x7fff810a54f0 sp 0x7fff810a54e8
READ of size 4 at 0x7f564317a4c8 thread T0
#0 0x7f56663d9455 in gfxShapedWord::CompressedGlyph::IsClusterStart() const ../../dist/include/gfxFont.h:1920
#1 0x7f56663225e9 in gfxTextRun::IsClusterStart(unsigned int) ../../dist/include/gfxFont.h:2383
#2 0x7f5666377101 in CountCharsFit(gfxTextRun*, unsigned int, unsigned int, double, PropertyProvider*, double*) layout/generic/nsTextFrameThebes.cpp:6128
#3 0x7f5666375ac4 in nsTextFrame::GetCharacterOffsetAtFramePointInternal(nsPoint const&, bool) layout/generic/nsTextFrameThebes.cpp:6171
#4 0x7f5666375047 in nsTextFrame::CalcContentOffsetsFromFramePoint(nsPoint) layout/generic/nsTextFrameThebes.cpp:6144
#5 0x7f5665f6051d in nsIFrame::GetContentOffsetsFromPoint(nsPoint, unsigned int) layout/generic/nsFrame.cpp:3643
#6 0x7f5665f5cde3 in nsFrame::HandlePress(nsPresContext*, nsGUIEvent*, nsEventStatus*) layout/generic/nsFrame.cpp:2714
#7 0x7f5665f56c27 in nsFrame::HandleEvent(nsPresContext*, nsGUIEvent*, nsEventStatus*) layout/generic/nsFrame.cpp:2401
#8 0x7f5665c259ae in nsPresShellEventCB::HandleEvent(nsEventChainPostVisitor&) layout/base/nsPresShell.cpp:492
#9 0x7f5668432cc3 in nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*, bool, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:362
#10 0x7f5668437f82 in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsIDOMEventTarget>*) content/events/src/nsEventDispatcher.cpp:629
#11 0x7f5665be315f in PresShell::HandleEventInternal(nsEvent*, nsEventStatus*) layout/base/nsPresShell.cpp:6474
#12 0x7f5665bdce63 in PresShell::HandlePositionedEvent(nsIFrame*, nsGUIEvent*, nsEventStatus*) layout/base/nsPresShell.cpp:6223
#13 0x7f5665bd4dbf in PresShell::HandleEvent(nsIFrame*, nsGUIEvent*, bool, nsEventStatus*) layout/base/nsPresShell.cpp:6022
#14 0x7f56699dc0f9 in nsViewManager::DispatchEvent(nsGUIEvent*, nsIView*, nsEventStatus*) view/src/nsViewManager.cpp:771
#15 0x7f56699c7817 in nsView::HandleEvent(nsGUIEvent*, bool) view/src/nsView.cpp:1068
#16 0x7f56699c7be1 in non-virtual thunk to nsView::HandleEvent(nsGUIEvent*, bool) :?
#17 0x7f566f348c7a in nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) widget/gtk2/nsWindow.cpp:458
#18 0x7f566f37c3b1 in nsWindow::OnButtonPressEvent(_GtkWidget*, _GdkEventButton*) widget/gtk2/nsWindow.cpp:2729
#19 0x7f566f393349 in button_press_event_cb(_GtkWidget*, _GdkEventButton*) widget/gtk2/nsWindow.cpp:5256
#20 0x7f566020cdd7 in ?? ??:0
0x7f564317a4c8 is located 0 bytes to the right of 136-byte region [0x7f564317a440,0x7f564317a4c8)
allocated by thread T0 here:
#0 0x4c3e10 in malloc ??:?
#1 0x7f5680d4b866 in moz_malloc memory/mozalloc/mozalloc.cpp:64
#2 0x7f56721dffbb in gfxTextRun::AllocateStorageForTextRun(unsigned long, unsigned int) gfx/thebes/gfxFont.cpp:4469
#3 0x7f56721c30dd in gfxTextRun::Create(gfxTextRunFactory::Parameters const*, unsigned int, gfxFontGroup*, unsigned int) gfx/thebes/gfxFont.cpp:4486
#4 0x7f56721c69f9 in gfxFontGroup::MakeTextRun(unsigned char const*, unsigned int, gfxTextRunFactory::Parameters const*, unsigned int) gfx/thebes/gfxFont.cpp:3531
#5 0x7f5666308b9e in gfxTextRun* MakeTextRun<unsigned char>(unsigned char const*, unsigned int, gfxFontGroup*, gfxTextRunFactory::Parameters const*, unsigned int) layout/generic/nsTextFrameThebes.cpp:552
#6 0x7f56662fd9fb in BuildTextRunsScanner::BuildTextRunForFrames(void*) layout/generic/nsTextFrameThebes.cpp:2049
#7 0x7f56662f2977 in BuildTextRunsScanner::FlushFrames(bool, bool) layout/generic/nsTextFrameThebes.cpp:1408
#8 0x7f5666316a21 in BuildTextRuns(gfxContext*, nsTextFrame*, nsIFrame*, nsLineList_iterator const*, nsTextFrame::TextRunType) layout/generic/nsTextFrameThebes.cpp:1336
Shadow byte and word:
0x1feac862f499: fb
0x1feac862f498: 00 fb fb fb fb fb fb fb
More shadow bytes:
0x1feac862f478: fd fd fd fd fd fd fd fd
0x1feac862f480: fa fa fa fa fa fa fa fa
0x1feac862f488: 00 00 00 00 00 00 00 00
0x1feac862f490: 00 00 00 00 00 00 00 00
=>0x1feac862f498: 00 fb fb fb fb fb fb fb
0x1feac862f4a0: fa fa fa fa fa fa fa fa
0x1feac862f4a8: fd fd fd fd fd fd fd fd
0x1feac862f4b0: fd fd fd fd fd fd fd fd
0x1feac862f4b8: fd fd fd fd fd fd fd fd
Stats: 268M malloced (261M for red zones) by 537106 calls
Stats: 44M realloced by 25642 calls
Stats: 237M freed by 307654 calls
Stats: 204M really freed by 253059 calls
Stats: 265M (67958 full pages) mmaped in 507 calls
mmaps by size class: 7:221130; 8:47081; 9:13299; 10:6643; 11:7650; 12:1664; 13:896; 14:544; 15:144; 16:680; 17:452; 18:134; 19:35; 20:21;
mallocs by size class: 7:380809; 8:92863; 9:25780; 10:10928; 11:16758; 12:3095; 13:1929; 14:1723; 15:343; 16:1260; 17:1370; 18:183; 19:42; 20:23;
frees by size class: 7:193336; 8:64604; 9:19790; 10:7821; 11:13658; 12:2141; 13:1717; 14:1555; 15:289; 16:1173; 17:1353; 18:158; 19:39; 20:20;
rfrees by size class: 7:163685; 8:49891; 9:15630; 10:4663; 11:12235; 12:1530; 13:1333; 14:1345; 15:244; 16:1098; 17:1289; 18:61; 19:37; 20:18;
Stats: malloc large: 3221 small slow: 5781
==18316== ABORTING
|
||
Updated•12 years ago
|
Component: General → Graphics
Product: Firefox → Core
|
Assignee | |
Comment 1•12 years ago
|
||
In a debug trunk build, this testcase starts by triggering a stream of assertions, beginning with
###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file /Users/jkew/mozdev/mozilla-inbound/gfx/thebes/gfxSkipChars.cpp, line 60
Finding the root cause triggering those assertions must be the place to start; if we've got an inconsistent or invalid gfxSkipChars, or we're calling it with out-of-range offsets and proceeding to use the results, a subsequent out-of-bounds access to textrun data is a natural result.
|
|
Assignee | |
Comment 2•12 years ago
|
||
Stack from the first assertion I hit when loading this testcase:
###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file /Users/jkew/mozdev/mozilla-inbound/gfx/thebes/gfxSkipChars.cpp, line 60
Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007fff8bcf482a in __kill ()
(gdb) bt
#0 0x00007fff8bcf482a in __kill ()
#1 0x000000010375fa52 in RealBreak () at /Users/jkew/mozdev/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:434
#2 0x000000010375f601 in Break (aMsg=0x7fff5fbf92f0 "###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file /Users/jkew/mozdev/mozilla-inbound/gfx/thebes/gfxSkipChars.cpp, line 60") at /Users/jkew/mozdev/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:546
#3 0x000000010375f174 in NS_DebugBreak_P (aSeverity=1, aStr=0x104e0da1c "Invalid offset", aExpr=0x104e0da2b "aOffset <= mSkipChars->mCharCount", aFile=0x104e0da4d "/Users/jkew/mozdev/mozilla-inbound/gfx/thebes/gfxSkipChars.cpp", aLine=60) at /Users/jkew/mozdev/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:415
#4 0x00000001038b4876 in gfxSkipCharsIterator::SetOffsets (this=0x7fff5fbf97a8, aOffset=6, aInOriginalString=true) at /Users/jkew/mozdev/mozilla-inbound/gfx/thebes/gfxSkipChars.cpp:59
#5 0x00000001019739b3 in gfxSkipCharsIterator::AdvanceOriginal (this=0x7fff5fbf97a8, aDelta=6) at gfxSkipChars.h:263
#6 0x0000000101968df1 in ComputeTransformedLength (aProvider=@0x7fff5fbf9aa0) at /Users/jkew/mozdev/mozilla-inbound/layout/generic/nsTextFrameThebes.cpp:5708
#7 0x0000000101963e6e in nsTextFrame::PaintText (this=0x111341718, aRenderingContext=0x117d72240, aPt=@0x7fff5fbf9c00, aDirtyRect=@0x7fff5fbf9c30, aItem=@0x1109d31b8, aCallbacks=0x0) at /Users/jkew/mozdev/mozilla-inbound/layout/generic/nsTextFrameThebes.cpp:5839
#8 0x0000000101963c59 in nsDisplayText::Paint (this=0x1109d31b8, aBuilder=0x7fff5fbfc268, aCtx=0x117d72240) at /Users/jkew/mozdev/mozilla-inbound/layout/generic/nsTextFrameThebes.cpp:4571
#9 0x00000001017320c9 in mozilla::FrameLayerBuilder::DrawThebesLayer (aLayer=0x1109d0800, aContext=0x1188188e0, aRegionToDraw=@0x7fff5fbfa420, aRegionToInvalidate=@0x7fff5fbfa4a8, aCallbackData=0x7fff5fbfc268) at /Users/jkew/mozdev/mozilla-inbound/layout/base/FrameLayerBuilder.cpp:3256
#10 0x00000001038ef44a in mozilla::layers::BasicThebesLayer::PaintBuffer (this=0x1109d0800, aContext=0x1188188e0, aRegionToDraw=@0x7fff5fbfa468, aExtendedRegionToDraw=@0x7fff5fbfa420, aRegionToInvalidate=@0x7fff5fbfa4a8, aDidSelfCopy=false, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268) at BasicThebesLayer.h:94
#11 0x00000001038ee2ac in mozilla::layers::BasicShadowableThebesLayer::PaintBuffer (this=0x1109d0800, aContext=0x1188188e0, aRegionToDraw=@0x7fff5fbfa468, aExtendedRegionToDraw=@0x7fff5fbfa420, aRegionToInvalidate=@0x7fff5fbfa4a8, aDidSelfCopy=false, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicThebesLayer.cpp:402
#12 0x00000001038ece77 in mozilla::layers::BasicThebesLayer::PaintThebes (this=0x1109d0800, aContext=0x110f5f1a0, aMaskLayer=0x0, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aReadback=0x7fff5fbfae00) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicThebesLayer.cpp:189
#13 0x00000001038eda1b in mozilla::layers::BasicShadowableThebesLayer::PaintThebes (this=0x1109d0800, aContext=0x110f5f1a0, aMaskLayer=0x0, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aReadback=0x7fff5fbfae00) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicThebesLayer.cpp:303
#14 0x00000001038edaff in non-virtual thunk to mozilla::layers::BasicShadowableThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) () at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicThebesLayer.cpp:314
#15 0x00000001038d9978 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren (this=0x118004500, aPaintContext=@0x7fff5fbfac38, aGroupTarget=0x110f5f1a0) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:823
#16 0x00000001038d8e42 in mozilla::layers::BasicLayerManager::PaintLayer (this=0x118004500, aTarget=0x110f5f1a0, aLayer=0x1109d0800, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aReadback=0x7fff5fbfae00) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:932
#17 0x00000001038d9ab4 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren (this=0x118004500, aPaintContext=@0x7fff5fbfb108, aGroupTarget=0x110f5f1a0) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:839
#18 0x00000001038d8e42 in mozilla::layers::BasicLayerManager::PaintLayer (this=0x118004500, aTarget=0x110f5f1a0, aLayer=0x1109d0400, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aReadback=0x7fff5fbfb2d0) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:932
#19 0x00000001038d9ab4 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren (this=0x118004500, aPaintContext=@0x7fff5fbfb5d8, aGroupTarget=0x110f5f1a0) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:839
#20 0x00000001038d8e42 in mozilla::layers::BasicLayerManager::PaintLayer (this=0x118004500, aTarget=0x110f5f1a0, aLayer=0x1109cfc00, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aReadback=0x0) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:932
#21 0x00000001038d804b in mozilla::layers::BasicLayerManager::EndTransactionInternal (this=0x118004500, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aFlags=mozilla::layers::LayerManager::END_NO_COMPOSITE) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:585
#22 0x00000001038d795a in mozilla::layers::BasicLayerManager::EndTransaction (this=0x118004500, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aFlags=mozilla::layers::LayerManager::END_NO_COMPOSITE) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:508
#23 0x00000001038daafc in mozilla::layers::BasicShadowLayerManager::EndTransaction (this=0x118004500, aCallback=0x101731670 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0x7fff5fbfc268, aFlags=mozilla::layers::LayerManager::END_NO_COMPOSITE) at /Users/jkew/mozdev/mozilla-inbound/gfx/layers/basic/BasicLayerManager.cpp:1130
#24 0x00000001017a52fc in nsDisplayList::PaintForFrame (this=0x7fff5fbfc240, aBuilder=0x7fff5fbfc268, aCtx=0x0, aForFrame=0x11027bc28, aFlags=13) at /Users/jkew/mozdev/mozilla-inbound/layout/base/nsDisplayList.cpp:1109
#25 0x00000001017a48df in nsDisplayList::PaintRoot (this=0x7fff5fbfc240, aBuilder=0x7fff5fbfc268, aCtx=0x0, aFlags=13) at /Users/jkew/mozdev/mozilla-inbound/layout/base/nsDisplayList.cpp:975
#26 0x00000001017efff1 in nsLayoutUtils::PaintFrame (aRenderingContext=0x0, aFrame=0x11027bc28, aDirtyRegion=@0x7fff5fbfcb08, aBackstop=4294967295, aFlags=772) at /Users/jkew/mozdev/mozilla-inbound/layout/base/nsLayoutUtils.cpp:1853
#27 0x0000000101824da1 in PresShell::Paint (this=0x10def8300, aViewToPaint=0x10dc48d80, aDirtyRegion=@0x7fff5fbfcb08, aFlags=129) at /Users/jkew/mozdev/mozilla-inbound/layout/base/nsPresShell.cpp:5340
#28 0x0000000102217115 in nsViewManager::ProcessPendingUpdatesForView (this=0x10e66f380, aView=0x10dc48d80, aFlushDirtyRegion=true) at /Users/jkew/mozdev/mozilla-inbound/view/src/nsViewManager.cpp:437
#29 0x0000000102219b97 in nsViewManager::ProcessPendingUpdates (this=0x10e66f380) at /Users/jkew/mozdev/mozilla-inbound/view/src/nsViewManager.cpp:1212
#30 0x00000001018404b8 in nsRefreshDriver::Notify (this=0x110b38000, aTimer=0x10e90bf20) at /Users/jkew/mozdev/mozilla-inbound/layout/base/nsRefreshDriver.cpp:432
#31 0x0000000103754a59 in nsTimerImpl::Fire (this=0x10e90bf20) at /Users/jkew/mozdev/mozilla-inbound/xpcom/threads/nsTimerImpl.cpp:475
#32 0x0000000103754dc6 in nsTimerEvent::Run (this=0x10b303038) at /Users/jkew/mozdev/mozilla-inbound/xpcom/threads/nsTimerImpl.cpp:555
#33 0x000000010374a67f in nsThread::ProcessNextEvent (this=0x10046f9d0, mayWait=false, result=0x7fff5fbfd173) at /Users/jkew/mozdev/mozilla-inbound/xpcom/threads/nsThread.cpp:620
#34 0x00000001036b46d2 in NS_ProcessPendingEvents_P (thread=0x10046f9d0, timeout=20) at nsThreadUtils.cpp:170
#35 0x000000010311690f in nsBaseAppShell::NativeEventCallback (this=0x100410ce0) at /Users/jkew/mozdev/mozilla-inbound/widget/xpwidgets/nsBaseAppShell.cpp:97
#36 0x00000001030a880c in nsAppShell::ProcessGeckoEvents (aInfo=0x100410ce0) at /Users/jkew/mozdev/mozilla-inbound/widget/cocoa/nsAppShell.mm:402
#37 0x00007fff884a04f1 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#38 0x00007fff8849fd5d in __CFRunLoopDoSources0 ()
#39 0x00007fff884c6b49 in __CFRunLoopRun ()
#40 0x00007fff884c6486 in CFRunLoopRunSpecific ()
#41 0x00007fff8a3304d3 in RunCurrentEventLoopInMode ()
#42 0x00007fff8a337781 in ReceiveNextEventCommon ()
#43 0x00007fff8a33760e in BlockUntilNextEventMatchingListInMode ()
#44 0x00007fff8706be31 in _DPSNextEvent ()
#45 0x00007fff8706b735 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#46 0x00000001030a6f97 in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x10b39bd70, _cmd=0x7fff879436b0, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff75886ae0, flag=1 '\001') at /Users/jkew/mozdev/mozilla-inbound/widget/cocoa/nsAppShell.mm:168
#47 0x00007fff87068071 in -[NSApplication run] ()
#48 0x00000001030a928c in nsAppShell::Run (this=0x100410ce0) at /Users/jkew/mozdev/mozilla-inbound/widget/cocoa/nsAppShell.mm:756
#49 0x0000000102d33b97 in nsAppStartup::Run (this=0x10b376ec0) at /Users/jkew/mozdev/mozilla-inbound/toolkit/components/startup/nsAppStartup.cpp:290
#50 0x000000010126edc8 in XREMain::XRE_mainRun (this=0x7fff5fbfeea8) at /Users/jkew/mozdev/mozilla-inbound/toolkit/xre/nsAppRunner.cpp:3799
#51 0x000000010126f555 in XREMain::XRE_main (this=0x7fff5fbfeea8, argc=1, argv=0x7fff5fbffa98, aAppData=0x1000081c0) at /Users/jkew/mozdev/mozilla-inbound/toolkit/xre/nsAppRunner.cpp:3866
#52 0x000000010126f97f in XRE_main (argc=1, argv=0x7fff5fbffa98, aAppData=0x1000081c0, aFlags=0) at /Users/jkew/mozdev/mozilla-inbound/toolkit/xre/nsAppRunner.cpp:3941
#53 0x0000000100001d3e in do_main (argc=1, argv=0x7fff5fbffa98) at /Users/jkew/mozdev/mozilla-inbound/browser/app/nsBrowserApp.cpp:174
#54 0x00000001000015b7 in main (argc=1, argv=0x7fff5fbffa98) at /Users/jkew/mozdev/mozilla-inbound/browser/app/nsBrowserApp.cpp:279
|
|
Assignee | |
Comment 3•12 years ago
|
||
The problem here arises when the nsTextFrame has a stale textrun that covers only 4 characters, but the script has mutated the content to be 6 chars long, leading to a mismatch between the PropertyProvider that expects to deal with a longer string than the gfxSkipChars we have. By passing true instead of false for aNotify when calling UpdateValueDisplay here, the text frame gets informed of the modification, and I don't hit the stream of assertions any more. Can we do this, or will it have other dire consequences?
Attachment #674634 -
Flags: review?(ehsan)
|
||
Updated•12 years ago
|
Assignee: nobody → jfkthame
|
||
Comment 4•12 years ago
|
||
Comment on attachment 674634 [details] [diff] [review]
notify in case textrun was already created
Review of attachment 674634 [details] [diff] [review]:
-----------------------------------------------------------------
This is sort of a wallpaper, but the editor broken-ness underlying the problem here would take a lot more than just this one patch to fix. Looking back at the history of this code it seems like I didn't have a very good reason not to notify in this case, and this testcase clearly shows that we should, so r=me if this doesn't break any tests.
Attachment #674634 -
Flags: review?(ehsan) → review+
|
|
Assignee | |
Comment 5•12 years ago
|
||
Comment on attachment 674634 [details] [diff] [review]
notify in case textrun was already created
[Security approval request comment]
How easily can the security issue be deduced from the patch? Not readily; it does not give any hint as to where the problem eventually occurs (in text-frame layout), or what circumstances trigger it.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. (We may want to create a crashtest from the testcase here, but that *would* provide a direct pointer to how to the problem can be triggered.)
Which older supported branches are affected by this flaw? This line of code is present at least as far back as esr10, so presumably all current branches are affected.
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? Trivial to create, minimal risk.
How likely is this patch to cause regressions; how much testing does it need? Seems unlikely to cause regressions; passed unit tests on tryserver.
Attachment #674634 -
Flags: sec-approval?
|
|
Assignee | |
Comment 6•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> How likely is this patch to cause regressions; how much testing does it
> need? Seems unlikely to cause regressions; passed unit tests on tryserver.
Apparently I spoke too soon. :( All unit tests passed on OS X, but it looks like it's triggering an a11y mochitest failure (just one!) on Linux and Windows. Though according to bug 781120, the test may have been fragile already. To be investigated further...
|
||
Comment 7•12 years ago
|
||
Can I get an answer to:
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? Trivial to create, minimal risk.
|
|
Assignee | |
Comment 8•12 years ago
|
||
This line of code is present at least as far back as esr10, so presumably all current branches are affected.
Backports - trivial to create, minimal risk. (It's a one-word change!)
However, note comment 6 - it appears this may be affecting an a11y test in an unexpected way, so that needs investigation before we can land it.
|
|
||
Comment 9•12 years ago
|
||
Ok. Once that is figured out, I'll give a sec-approval+
|
|
Assignee | |
Comment 10•12 years ago
|
||
The test failure this causes comes from accessible/tests/mochitest/textcaret/test_general.html, which includes the code:
// test caret move events and caret offsets
gQueue = new eventQueue();
gQueue.push(new setCaretOffset("textbox", 1, "textbox"));
gQueue.push(new setCaretOffset("link", 1, "link"));
gQueue.push(new setCaretOffset("heading", 1, document));
gQueue.onFinish = function()
{
turnCaretBrowsing(false);
}
gQueue.invoke(); // Will call SimpleTest.finish();
}
where the document being used includes:
<input id="textbox" value="hello"/>
<textarea id="textarea">text<br>text</textarea>
<p id="p" contentEditable="true"><span>text</span><br/>text</p>
<a id="link" href="about:">about mozilla</a>
<h5 id="heading">heading</h5>
The error comes from the setCaretOffset operation targeting the textbox, which reports:
ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/textcaret/test_general.html | Wrong caret offset for ['input@id="textbox" node', address: [object HTMLInputElement], role: entry, address: 0x277a010] - got 5, expected 1
suggesting that the caret has ended up at the end of the textbox contents, instead of at offset 1 as expected.
This only occurs on the *first* setCaretOffset targeting the textbox; if I interleave further setCaretOffset(textbox,...) tests after the ones targeting the link and heading elements, they work correctly, regardless of the offset used - whether zero, 5, or anything in between.
So it's not that setCaretOffset is completely broken; what I guess may be happening is that the editor is only initialized by that first setCaretOffset call, and when this notification is fired during initialization, it's causing the selection to be moved to the end of the textbox content, so we lose the positioning that setCaretOffset just tried to do.
Ehsan, any suggestions how to overcome/work around/prevent this? Preferably without diving deep into editor...
|
|
||
Comment 11•12 years ago
|
||
Hmm, this might have something to do with the editor being initialized too late. Try setting initEagerly to true unconditionally here: <http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsTextControlFrame.cpp#389>, and see if the problem goes away.
|
|
||
Comment 12•12 years ago
|
||
(The problem being the test failure)
|
|
Assignee | |
Comment 13•12 years ago
|
||
Yes, that fixes the test failure, at least in my local build.
So how bad would it be if we do that, just removing the non-eager option there? I notice that the code already checks "initEagerly = txtCtrl->HasCachedSelection();", so that it'll be eager if there's a cached selection, which seems somewhat related to the problem case here. The key difference being that in the a11y testcase, it's not a cached selection, so this doesn't detect it; it's a selection that is being set after the decision to be lazy, but before the editor actually gets initialized.
|
|
Assignee | |
Comment 14•12 years ago
|
||
Hmm, while forcing the editor to init eagerly fixes the failure with the a11y test above, it leads to other mochitest failures... see https://tbpl.mozilla.org/?tree=Try&rev=c66217b05e6e.
|
||
Comment 15•12 years ago
|
||
Comment on attachment 674634 [details] [diff] [review]
notify in case textrun was already created
Sounds like we're not ready to check this in. Clearing sec-approval flag for now, please re-add it when you've got a patch that passes tests. If the same answers apply feel free to skip the sec-approval comment questionnaire.
Attachment #674634 -
Flags: sec-approval?
|
|
||
Comment 16•12 years ago
|
||
Well, we definitely don't want to init all editors eagerly. I was just trying to figure out what the root cause of the problem is. It seems like something in the a11y engine is relying on the editor being initialized eagerly without actually asking for it. CCing David to see if he can help us detect what exactly needs to be done on the a11y side.
|
|
||
Comment 17•12 years ago
|
||
Critsmash Triag: Can someone suggest a security rating here?
Whiteboard: [asan]
|
||
Comment 18•12 years ago
|
||
ok, so here's the interesting bits of setCaretOffset()
function setCaretOffset(aID, aOffset, aFocusTargetID)
{
this.target = getAccessible(aID, [nsIAccessibleText]);
this.offset = aOffset == -1 ? this.target.characterCount: aOffset;
this.focus = aFocusTargetID ? getAccessible(aFocusTargetID) : null;
this.invoke = function setCaretOffset_invoke()
{
this.target.caretOffset = this.offset;
}
so we're calling nsIAccessibleText.caretOffset's setter which is HyperTextAccessible::SetCaretOffset() which just calls HyperTextAccessible::SetSelectionRange() see accessible/src/generic/HyperTextAccessible.cpp
I think it should roughly do this
first ask the focus manager to focus the input id="textbox"
then get the frame selection with input->GetPrimaryFrame()->GetFrameSelection()
get the normal selection for that frame selection.
grab the first range from that selection set its start and end bounds to be the caret offset in this input then add and reinsert the range into the selection.
|
|
||
Comment 19•12 years ago
|
||
Hmm, shouldn't HyperTextAccessible attempt to initialize the editor first?
|
|
||
Comment 20•12 years ago
|
||
(In reply to Al Billings [:abillings] from comment #17)
> Critsmash Triag: Can someone suggest a security rating here?
It's a buffer overflow. Presumably a weaponized version can write stuff on the heap and combine that with other attacks to do something interesting.
|
|
||
Comment 21•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #19)
> Hmm, shouldn't HyperTextAccessible attempt to initialize the editor first?
if you say so? that code is mostly from before my time. I'd guess that should only happen on editable things? How does one try and initialize the editor?
would it make more sense to just have this test force the editor to insantiate, and fix a11y in another bug? I'm not sure how hard this is.
|
|
||
Comment 22•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #21)
> (In reply to Ehsan Akhgari [:ehsan] from comment #19)
> > Hmm, shouldn't HyperTextAccessible attempt to initialize the editor first?
>
> if you say so? that code is mostly from before my time. I'd guess that
> should only happen on editable things? How does one try and initialize the
> editor?
Hmm, well, I really don't know what that class is supposed to do, but if it's messing with the caret without an editor being around, then that's bad. You should be able to just call GetEditor() on the input element, which initializes the editor if it's not already initialized.
> would it make more sense to just have this test force the editor to
> insantiate, and fix a11y in another bug? I'm not sure how hard this is.
Hmm, yeah I guess we could do that too. In order to init the editor in the test, you should just be able to call .focus() on it (and also .blur() it if the editor having focus screws up the test results, etc.)
|
|
||
Comment 23•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #22)
> (In reply to Trevor Saunders (:tbsaunde) from comment #21)
> > (In reply to Ehsan Akhgari [:ehsan] from comment #19)
> > > Hmm, shouldn't HyperTextAccessible attempt to initialize the editor first?
> >
> > if you say so? that code is mostly from before my time. I'd guess that
> > should only happen on editable things? How does one try and initialize the
> > editor?
>
> Hmm, well, I really don't know what that class is supposed to do, but if
> it's messing with the caret without an editor being around, then that's bad.
> You should be able to just call GetEditor() on the input element, which
> initializes the editor if it's not already initialized.
Well, that class is basically for generic html type nodes that might contain text either editable or not. I'm not really sure why it currently tries to set the caret in non editable things, maybe for the caret navigation thing?? I'm not really sure. the platform api we expose to screen readers etc has a concept of caret in non editable things iirc.
> > would it make more sense to just have this test force the editor to
> > insantiate, and fix a11y in another bug? I'm not sure how hard this is.
>
> Hmm, yeah I guess we could do that too. In order to init the editor in the
> test, you should just be able to call .focus() on it (and also .blur() it if
> the editor having focus screws up the test results, etc.)
if initializing the editor is easy as it seems then I'd prefer that, but would probably take either.
|
|
||
Comment 24•12 years ago
|
||
OK, let's just hack around this in the test then. Thanks for your help, Trevor!
|
|
||
Comment 25•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #24)
> OK, let's just hack around this in the test then. Thanks for your help,
> Trevor!
np, I filed 807541 to fix this properly
|
|
Assignee | |
Comment 26•12 years ago
|
||
In my local testing, it looks like adding GetEditor() at the beginning of HyperTextAccessible::SetSelectionRange is sufficient to deal with this and avoid the test failure. I've pushed a tryserver job to check... https://tbpl.mozilla.org/?tree=Try&rev=a806caa5b4af.
|
|
Assignee | |
Comment 27•12 years ago
|
||
We could hack around the issue by modifying the testcase, but it seems simplest to just add the editor initialization here, if that's OK with you, Trevor. It looks like the try job is coming up nice and green.
Attachment #677303 -
Flags: review?(trev.saunders)
|
||
Comment 28•12 years ago
|
||
caretOffset makes editable area focused what should initialize the editor. Basically it seems accessibility doesn't make things impossible in js. If that's security issue then the patch shouldn't solve a problem in general.
|
|
||
Updated•12 years ago
|
Keywords: sec-critical
|
||
Comment 29•12 years ago
|
||
jfkthame, what FF versions does this bug affect? (Please mark status flags if you can)
|
|
Assignee | |
Comment 30•12 years ago
|
||
Verifying all the affected FF versions would require a bunch of ASAN builds (which I don't have on hand). As noted in comments 5 and 8, the relevant code has been present for a long time (at least as far back as esr10), so it seems likely all current versions are affected. Inspection shows no significant changes in this area between esr10 and current trunk code, so I expect the same flaw to exist and the same patch to fix it.
Marking "affected" for all versions, but this is based on code inspection rather than actual testing.
status-firefox-esr10:
--- → affected
status-firefox16:
--- → affected
status-firefox17:
--- → affected
status-firefox18:
--- → affected
status-firefox19:
--- → affected
status-firefox-esr17:
--- → affected
|
|
||
Comment 31•12 years ago
|
||
Comment on attachment 677303 [details] [diff] [review]
pt 2 - ensure editor is initialized if necessary before trying to set selection range in HyperTextAccessible
> HyperTextAccessible::SetSelectionRange(int32_t aStartPos, int32_t aEndPos)
> {
>+ // ensure editor is initialized if necessary
>+ nsCOMPtr<nsIEditor> editor = GetEditor();
wouldn't it be better to do this after TakeFocus() incase focusing the element somehow makes the editor we just forced to exist invalid? I'm not sure if that's an actual posibility or not, ehsan?
either way this seems fine.
Attachment #677303 -
Flags: review?(trev.saunders) → review+
|
|
||
Comment 32•12 years ago
|
||
(In reply to alexander :surkov from comment #28)
> caretOffset makes editable area focused what should initialize the editor.
from comment 18 and ehsan's response in comment 19 it seems that might not be enough.
> Basically it seems accessibility doesn't make things impossible in js. If
> that's security issue then the patch shouldn't solve a problem in general.
I'm unclear on what your trying to say here
|
|
||
Comment 33•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #32)
> (In reply to alexander :surkov from comment #28)
> > caretOffset makes editable area focused what should initialize the editor.
>
> from comment 18 and ehsan's response in comment 19 it seems that might not
> be enough.
A bridge from comment #18 to comment #19 is missed in my mind. If Eshan can give details then it'd be great.
> > Basically it seems accessibility doesn't make things impossible in js. If
> > that's security issue then the patch shouldn't solve a problem in general.
>
> I'm unclear on what your trying to say here
I assumed the problem is because of caretOffset. I said it seems you can do in js anything what caretOffset does. If that was true then a fix in a11y shouldn't fix the security issue (because everybody can do the same in js without a11y).
But this idea doesn't goes with the stack from bug description since I don't see there caretOffset (btw, I don't see a11y there at all).
Can you give me an idea what happens here?
|
|
||
Comment 34•12 years ago
|
||
(In reply to alexander :surkov from comment #33)
> (In reply to Trevor Saunders (:tbsaunde) from comment #32)
> > (In reply to alexander :surkov from comment #28)
> > > caretOffset makes editable area focused what should initialize the editor.
> >
> > from comment 18 and ehsan's response in comment 19 it seems that might not
> > be enough.
>
> A bridge from comment #18 to comment #19 is missed in my mind. If Eshan can
> give details then it'd be great.
>
> > > Basically it seems accessibility doesn't make things impossible in js. If
> > > that's security issue then the patch shouldn't solve a problem in general.
> >
> > I'm unclear on what your trying to say here
>
> I assumed the problem is because of caretOffset. I said it seems you can do
> in js anything what caretOffset does. If that was true then a fix in a11y
> shouldn't fix the security issue (because everybody can do the same in js
> without a11y).
>
> But this idea doesn't goes with the stack from bug description since I don't
> see there caretOffset (btw, I don't see a11y there at all).
>
> Can you give me an idea what happens here?
So, as I understand it there's some bug or another completely unrelated to a11y that involves notifying something or another related to text runs about a change. Fixing that apparently changes when the editor is instatiated in the test accessible/test/textcaret/test_general.html so that the test no longer passes see earlier discussion in the bug, aparently we set the caret offset in a11y then the editor is started before the test, and that causes the caret to be relocated to the end of the input.
|
|
||
Comment 35•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #34)
> > Can you give me an idea what happens here?
>
> So, as I understand it there's some bug or another completely unrelated to
> a11y that involves notifying something or another related to text runs about
> a change. Fixing that apparently changes when the editor is instatiated in
> the test accessible/test/textcaret/test_general.html so that the test no
> longer passes see earlier discussion in the bug, aparently we set the caret
> offset in a11y then the editor is started before the test, and that causes
> the caret to be relocated to the end of the input.
It seems it's a bug and the editor instantiation on a11y side is hacky workaround.
|
|
||
Comment 36•12 years ago
|
||
(In reply to alexander :surkov from comment #35)
> (In reply to Trevor Saunders (:tbsaunde) from comment #34)
>
> > > Can you give me an idea what happens here?
> >
> > So, as I understand it there's some bug or another completely unrelated to
> > a11y that involves notifying something or another related to text runs about
> > a change. Fixing that apparently changes when the editor is instatiated in
> > the test accessible/test/textcaret/test_general.html so that the test no
> > longer passes see earlier discussion in the bug, aparently we set the caret
> > offset in a11y then the editor is started before the test, and that causes
> > the caret to be relocated to the end of the input.
>
> It seems it's a bug and the editor instantiation on a11y side is hacky
> workaround.
maybe? talk to ehsan about that not me though :) On the other hand I'm sort of guessing moving around input with keys also forces the editor to be instantiated in which case a11y is sort of programatically doing what keyboard input does, so then it seems reasonable that both should do same steps.
|
|
Assignee | |
Comment 37•12 years ago
|
||
Combined the two parts into a single patch for landing. Carrying forward r=ehsan,tbsaunde.
|
|
Assignee | |
Updated•12 years ago
|
Attachment #674634 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•12 years ago
|
Attachment #677303 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 38•12 years ago
|
||
Comment on attachment 677679 [details] [diff] [review]
Folded patch ready for landing.
[Security approval request comment]
See comment 5.
The original fix in nsTextEditorState is unchanged, and the notes in comment 5 still apply. The added fix in HyperTextAccessible is not actually a security issue but is needed because the change to editor initialization behavior will otherwise cause a mochitest failure.
Attachment #677679 -
Flags: sec-approval?
|
|
||
Comment 39•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #38)
> The added fix in HyperTextAccessible is not actually a
> security issue but is needed because the change to editor initialization
> behavior will otherwise cause a mochitest failure.
It seems the problem is not a failing mochitest, the problem is the developer can't change a caret under certain circumstances (comment #34). However I'm fine to have a workaround on a11y side to keep a11y working but it doesn't make a good job for web developers.
btw, the comment doesn't make sense. If you don't have an access to this bug then you can't get a good idea why we were needed to initialize the editor there. Please make the comment descriptive.
|
|
Assignee | |
Comment 40•12 years ago
|
||
OK, suggested replacement comment:
// Before setting the selection range, we need to ensure that the editor
// is initialized. (See bug 804927.)
// Otherwise, it's possible that lazy editor initialization will override
// the selection we set here and leave the caret at the end of the text.
// By calling GetEditor here, we ensure that editor initialization is
// completed before we set the selection.
Better?
|
|
||
Comment 41•12 years ago
|
||
(In reply to alexander :surkov from comment #33)
> (In reply to Trevor Saunders (:tbsaunde) from comment #32)
> > (In reply to alexander :surkov from comment #28)
> > > caretOffset makes editable area focused what should initialize the editor.
> >
> > from comment 18 and ehsan's response in comment 19 it seems that might not
> > be enough.
>
> A bridge from comment #18 to comment #19 is missed in my mind. If Eshan can
> give details then it'd be great.
Not sure what more details I can provide here. The HyperTextAccessible object seems to try to adjust the selection on input elements without first making sure that the editor is initialized, and that's broken. Jonathan's patch correctly fixes that.
|
|
||
Updated•12 years ago
|
Attachment #677679 -
Flags: sec-approval? → sec-approval+
|
|
||
Comment 42•12 years ago
|
||
Jonathan, please don't mention editor initialization in the commit message. Just say "fix text control frame and accessibility issues" or something. :-)
|
|
Assignee | |
Comment 43•12 years ago
|
||
Target Milestone: --- → mozilla19
|
|
||
Comment 44•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #40)
> OK, suggested replacement comment:
>
> // Before setting the selection range, we need to ensure that the editor
> // is initialized. (See bug 804927.)
> // Otherwise, it's possible that lazy editor initialization will override
> // the selection we set here and leave the caret at the end of the text.
> // By calling GetEditor here, we ensure that editor initialization is
> // completed before we set the selection.
>
> Better?
It's nice. Thank you.
|
|
||
Comment 45•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #41)
> (In reply to alexander :surkov from comment #33)
> > (In reply to Trevor Saunders (:tbsaunde) from comment #32)
> > > (In reply to alexander :surkov from comment #28)
> > > > caretOffset makes editable area focused what should initialize the editor.
> > >
> > > from comment 18 and ehsan's response in comment 19 it seems that might not
> > > be enough.
> >
> > A bridge from comment #18 to comment #19 is missed in my mind. If Eshan can
> > give details then it'd be great.
>
> Not sure what more details I can provide here. The HyperTextAccessible
> object seems to try to adjust the selection on input elements without first
> making sure that the editor is initialized, and that's broken.
yes, thank you
> Jonathan's
> patch correctly fixes that.
my concerns was that a11y doesn't make any things that can't be done in js and that means that the web developer might run into troubles of changing the selection.
|
|
Assignee | |
Comment 46•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
|
|
||
Comment 47•12 years ago
|
||
(In reply to alexander :surkov from comment #45)
> (In reply to Ehsan Akhgari [:ehsan] from comment #41)
> my concerns was that a11y doesn't make any things that can't be done in js
> and that means that the web developer might run into troubles of changing
> the selection.
No, we already take care of this in the web facing APIs.
|
|
||
Comment 48•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #47)
> (In reply to alexander :surkov from comment #45)
> > (In reply to Ehsan Akhgari [:ehsan] from comment #41)
> > my concerns was that a11y doesn't make any things that can't be done in js
> > and that means that the web developer might run into troubles of changing
> > the selection.
>
> No, we already take care of this in the web facing APIs.
Ok, then I don't mind.
|
||
Comment 49•12 years ago
|
||
Please nominate this sg:crit for uplift to FF17 and ESR10 asap.
|
|
Assignee | |
Comment 50•12 years ago
|
||
Comment on attachment 677679 [details] [diff] [review]
Folded patch ready for landing.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): no regression (long-standing bug)
User impact if declined: potential for out-of-bounds access in textrun code
Testing completed (on m-c, etc.): on m-c for several days, no issues
Risk to taking this patch (and alternatives if risky): low risk - no major behavior change, just adds notification to ensure textrun is updated if necessary during editor setup to avoid risk of using stale/mismatched data
String or UUID changes made by this patch: none
Attachment #677679 -
Flags: approval-mozilla-beta?
Attachment #677679 -
Flags: approval-mozilla-aurora?
|
|
Assignee | |
Comment 51•12 years ago
|
||
Rebased patch for esr10.
|
|
Assignee | |
Comment 52•12 years ago
|
||
Comment on attachment 680019 [details] [diff] [review]
fix text control frame and accessibility issues.
[Approval Request Comment]
As above - patch just needed minor rebasing to apply on esr10.
Attachment #680019 -
Flags: approval-mozilla-esr10?
|
|
||
Comment 53•12 years ago
|
||
Comment on attachment 677679 [details] [diff] [review]
Folded patch ready for landing.
We're going to build with our final FF17 beta on Monday. Please land asap. Thanks!
Attachment #677679 -
Flags: approval-mozilla-beta?
Attachment #677679 -
Flags: approval-mozilla-beta+
Attachment #677679 -
Flags: approval-mozilla-aurora?
Attachment #677679 -
Flags: approval-mozilla-aurora+
|
|
||
Updated•12 years ago
|
Attachment #680019 -
Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
|
|
Assignee | |
Comment 54•12 years ago
|
||
|
|
Assignee | |
Comment 55•12 years ago
|
||
Corrected esr10 patch - there were additional changes in a11y code that require adjustments to the patch here.
|
|
Assignee | |
Updated•12 years ago
|
Attachment #680019 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 56•12 years ago
|
||
|
|
||
Updated•12 years ago
|
Alias: CVE-2012-5839
Whiteboard: [asan] → [asan][adv-track-main17+][adv-track-esr17+]
|
|
||
Updated•12 years ago
|
|
|
||
Updated•12 years ago
|
|
|
||
Comment 57•12 years ago
|
||
This bug qualifies for a security bug bounty.
Flags: sec-bounty? → sec-bounty+
|
|
||
Updated•12 years ago
|
Group: core-security
|
||
Updated•9 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•