709580 - Out of bounds access in GfxInfoBase::GetFeatureStatusImpl

Status: RESOLVED DUPLICATE of bug 706702

(Core :: Graphics, defect)

Description

[Christian Holler (:decoder)]

Address sanitizer reports me the following error on startup of Firefox:

==3162== ERROR: AddressSanitizer global-buffer-overflow on address 0x7f014a6e0978 at pc 0x7f013fdc1097 bp 0x7fff5bc12870 sp 0x7fff5bc12868
READ of size 4 at 0x7f014a6e0978 thread T0
    #0 0x7f013fdc1097 in _ZN7mozilla6widget11GfxInfoBase20GetFeatureStatusImplEiPiR18nsAString_internalPNS0_13GfxDriverInfoEPNS0_15OperatingSystemE /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/xpwidgets/GfxInfoBase.cpp:626
    #1 0x7f013fdf1b2c in _ZN7mozilla6widget7GfxInfo20GetFeatureStatusImplEiPiR18nsAString_internalPNS0_13GfxDriverInfoEPNS0_15OperatingSystemE /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/xpwidgets/GfxInfoX11.cpp:322
    #2 0x7f013fdbdf12 in _ZN7mozilla6widget11GfxInfoBase16GetFeatureStatusEiPi /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/xpwidgets/GfxInfoBase.cpp:574
    #3 0x7f013fd0a50c in _ZN12nsBaseWidget19GetShouldAccelerateEv /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/xpwidgets/nsBaseWidget.cpp:807
    #4 0x7f013fd0b48e in _ZN12nsBaseWidget15GetLayerManagerEPN7mozilla6layers12PLayersChildENS1_12LayerManager13LayersBackendEN9nsIWidget23LayerManagerPersistenceEPb /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/xpwidgets/nsBaseWidget.cpp:839
    #5 0x7f013fb6d539 in _ZN8nsWindow13OnExposeEventEP15_GdkEventExpose /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/gtk2/nsWindow.cpp:2213
    #6 0x7f013fba5f70 in expose_event_cb /home/decoder/LangFuzz/mozilla-central-llvm/widget/src/gtk2/nsWindow.cpp:5712
    #7 0x7f0130cc1578 in ?? ??:0
0x7f014a6e0978 is located 0 bytes to the right of global variable '_ZN7mozilla6widgetL11gDriverInfoE' (0x7f014a6e0940) of size 56
  '_ZN7mozilla6widgetL11gDriverInfoE' is ascii string ''
==3162== ABORTING


Closer investigation shows that this is due to the access |if (!info->mOperatingSystem) { | in line 626 being out of bounds. The loop starts and does the call to GetGfxDriverInfo in line 634 (which returns | &gDriverInfo[0] | while the whole gDriverInfo array has size 1). Right after that call, the | info++ | in line 641 is executed, leaving info being invalid.

bjacob suggested that this can be fixed by adding a | continue; | right after | info = GetGfxDriverInfo(); | and I can confirm this fixes the issue.

Comment 1

[Christian Holler (:decoder)]

Attachment: Patch

Fix as described in comment 0 and suggested by bjacob.

Comment 2

[Doug Sherk (:drs) (inactive)]

Comment on attachment 581128 [details] [diff] [review]
Patch

r+, but I will be landing changes that will make this code irrelevant probably tomorrow or Wednesday. I think with the tree close, it might not be worth landing this.

Comment 3

[Christian Holler (:decoder)]

Okay, seems fine to me. Once you land your changes and they fix the issue, can you mark this as WFM/DUPLICATE to the other bug so people checking this bug know it's no longer an issue? (I included this bug in one my manuals).

Comment 4

[Doug Sherk (:drs) (inactive)]

Sure. For reference, it will be part of the stack of bugs in bug 706702.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Christian Holler (:decoder) from comment #3)
> Okay, seems fine to me. Once you land your changes and they fix the issue,
> can you mark this as WFM/DUPLICATE to the other bug so people checking this
> bug know it's no longer an issue? (I included this bug in one my manuals).

Why not FIXED ??

Comment 6

[Doug Sherk (:drs) (inactive)]

I agree with DUPLICATE, FIXED would imply that the fix was part of the patch on this bug report.

Comment 8

[Doug Sherk (:drs) (inactive)]

Thanks, forgot to do that.