Closed Bug 1840088 Opened 1 year ago Closed 1 year ago

AddressSanitizer: heap-buffer-overflow [@ GetIntegerValue] with READ of size 4

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
116 Branch
Tracking Flags:
Tracking Status
firefox-esr102 --- unaffected
firefox-esr115 --- unaffected
firefox114 --- unaffected
firefox115 --- unaffected
firefox116 + fixed

People

(Reporter: jkratzer, Assigned: mccr8)

References

(Blocks 1 open bug, Regression)

Details

(4 keywords, Whiteboard: [bugmon:bisected,confirmed][fuzzblocker])

Crash Data

Attachments

(4 files)

Detailed Crash Information
12.78 KB, text/plain
Details
Testcase
42 bytes, text/plain
Details
Bug 1840088 - Add back type check to HTMLTableElement::BuildInheritedAttributes().
48 bytes, text/x-phabricator-request
Details | Review
Bug 1840088 - Crash test.
48 bytes, text/x-phabricator-request
Details | Review

Testcase found while fuzzing mozilla-central rev 750c24176cc2 (built with: --enable-address-sanitizer --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build 750c24176cc2 --asan --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

AddressSanitizer: heap-buffer-overflow [@ GetIntegerValue] with READ of size 4

    =================================================================
    ==3815073==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000013430 at pc 0x7f634cce325e bp 0x7fffd699dab0 sp 0x7fffd699daa8
    READ of size 4 at 0x603000013430 thread T0 (Isolated Web Co)
        #0 0x7f634cce325d in GetIntegerValue /dom/base/nsAttrValueInlines.h:148:68
        #1 0x7f634cce325d in mozilla::dom::HTMLTableElement::BuildInheritedAttributes() /dom/html/HTMLTableElement.cpp:943:28
        #2 0x7f634cce32da in mozilla::dom::HTMLTableElement::BindToTree(mozilla::dom::BindContext&, nsINode&) /dom/html/HTMLTableElement.cpp:960:3
        #3 0x7f634922467d in nsINode::InsertChildBefore(nsIContent*, nsIContent*, bool, mozilla::ErrorResult&) /dom/base/nsINode.cpp:1612:15
        #4 0x7f63475dcdfc in AppendChildTo /dom/base/nsINode.h:931:5
        #5 0x7f63475dcdfc in nsHtml5TreeOperation::Append(nsIContent*, nsIContent*, nsHtml5DocumentBuilder*) /parser/html/nsHtml5TreeOperation.cpp:253:12
        #6 0x7f63475c99a6 in nsHtml5TreeOperation::Append(nsIContent*, nsIContent*, mozilla::dom::FromParser, nsHtml5DocumentBuilder*) /parser/html/nsHtml5TreeOperation.cpp:270:17
        #7 0x7f63475d3253 in operator() /parser/html/nsHtml5TreeOperation.cpp:798:14
        #8 0x7f63475d3253 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opUpdateCharsetSource, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineAndColumnNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> &> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:266:16
        #9 0x7f63475d3253 in match<TreeOperationMatcher, mozilla::Variant<uninitialized, opAppend, opDetach, opAppendChildrenToNewParent, opFosterParent, opAppendToDocument, opAddAttributes, nsHtml5DocumentMode, opCreateHTMLElement, opCreateSVGElement, opCreateMathMLElement, opSetFormElement, opAppendText, opFosterParentText, opAppendComment, opAppendCommentToDocument, opAppendDoctypeToDocument, opGetDocumentFragmentForTemplate, opGetFosterParent, opMarkAsBroken, opRunScript, opRunScriptAsyncDefer, opPreventScriptExecution, opDoneAddingChildren, opDoneCreatingElement, opUpdateCharsetSource, opCharsetSwitchTo, opUpdateStyleSheet, opProcessOfflineManifest, opMarkMalformedIfScript, opStreamEnded, opSetStyleLineNumber, opSetScriptLineAndColumnNumberAndFreeze, opSvgLoad, opMaybeComplainAboutCharset, opMaybeComplainAboutDeepTree, opAddClass, opAddViewSourceHref, opAddViewSourceBase, opAddErrorType, opAddLineNumberId, opStartLayout, opEnableEncodingMenu> &> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:279:14
        #10 0x7f63475d3253 in match<TreeOperationMatcher> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:852:12
        #11 0x7f63475d3253 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*) /parser/html/nsHtml5TreeOperation.cpp:1206:21
        #12 0x7f63475d1ed2 in nsHtml5TreeOpExecutor::RunFlushLoop() /parser/html/nsHtml5TreeOpExecutor.cpp:694:19
        #13 0x7f63475df722 in nsHtml5ExecutorFlusher::Run() /parser/html/nsHtml5StreamParser.cpp:174:18
        #14 0x7f6344fa753a in mozilla::RunnableTask::Run() /xpcom/threads/TaskController.cpp:555:16
        #15 0x7f6344f924ce in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:880:26
        #16 0x7f6344f8f417 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:704:15
        #17 0x7f6344f8fcff in mozilla::TaskController::ProcessPendingMTTask(bool) /xpcom/threads/TaskController.cpp:491:36
        #18 0x7f6344faf171 in operator() /xpcom/threads/TaskController.cpp:218:37
        #19 0x7f6344faf171 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /xpcom/threads/nsThreadUtils.h:548:5
        #20 0x7f6344fd9283 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1199:16
        #21 0x7f6344fe6cb4 in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:479:10
        #22 0x7f6346bfcdee in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:85:21
        #23 0x7f6346a24e9a in RunInternal /ipc/chromium/src/base/message_loop.cc:370:10
        #24 0x7f6346a24e9a in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #25 0x7f6346a24e9a in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #26 0x7f6350196839 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:148:27
        #27 0x7f635602179e in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:717:20
        #28 0x7f6346a24e9a in RunInternal /ipc/chromium/src/base/message_loop.cc:370:10
        #29 0x7f6346a24e9a in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #30 0x7f6346a24e9a in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #31 0x7f6356020e58 in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:652:34
        #32 0x55f51d4fb6de in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #33 0x55f51d4fb6de in main /browser/app/nsBrowserApp.cpp:375:18
        #34 0x7f636bacdd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #35 0x7f636bacde3f in __libc_start_main csu/../csu/libc-start.c:392:3
        #36 0x55f51d424d18 in _start (/home/jkratzer/builds/m-c-20230623092529-fuzzing-asan-opt/firefox+0x107d18) (BuildId: 634671658158d494eb6237b942b82ef19c5d8ff5)
    
    0x603000013432 is located 0 bytes after 18-byte region [0x603000013420,0x603000013432)
    allocated by thread T4 here:
        #0 0x55f51d4bd64e in malloc /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
        #1 0x7f6344cc6ced in nsStringBuffer::Alloc(unsigned long) /xpcom/string/nsStringBuffer.cpp:68:42
        #2 0x7f6347549382 in nsHtml5String::FromBuffer(char16_t*, int, nsHtml5TreeBuilder*) /parser/html/nsHtml5String.cpp:113:7
        #3 0x7f6347514699 in nsHtml5Portability::newStringFromBuffer(char16_t*, int, int, nsHtml5TreeBuilder*, bool) /parser/html/nsHtml5Portability.cpp:47:10
        #4 0x7f6347562533 in strBufToString /parser/html/nsHtml5Tokenizer.cpp:246:23
        #5 0x7f6347562533 in nsHtml5Tokenizer::addAttributeWithValue() /parser/html/nsHtml5Tokenizer.cpp:389:25
        #6 0x7f634758e1a4 in int nsHtml5Tokenizer::stateLoop<nsHtml5SilentPolicy>(int, char16_t, int, char16_t*, bool, int, int) /parser/html/nsHtml5Tokenizer.cpp:908:15
        #7 0x7f634754352d in nsHtml5Tokenizer::tokenizeBuffer(nsHtml5UTF16Buffer*) /parser/html/nsHtml5Tokenizer.cpp:450:11
        #8 0x7f63475332c7 in nsHtml5StreamParser::ParseAvailableData() /parser/html/nsHtml5StreamParser.cpp:2476:32
        #9 0x7f63475304c8 in nsHtml5StreamParser::DoDataAvailable(mozilla::Span<unsigned char const, 18446744073709551615ul>) /parser/html/nsHtml5StreamParser.cpp:1564:3
        #10 0x7f63475355b1 in nsHtml5StreamParser::DoDataAvailableBuffer(mozilla::Buffer<unsigned char>&&) /parser/html/nsHtml5StreamParser.cpp:1482:5
        #11 0x7f63475226ad in nsHtml5StreamParser::OnDataAvailable(nsIRequest*, nsIInputStream*, unsigned long, unsigned int) /parser/html/nsHtml5StreamParser.cpp:1658:7
        #12 0x7f634752240a in nsHtml5StreamListener::OnDataAvailable(nsIRequest*, nsIInputStream*, unsigned long, unsigned int) /parser/html/nsHtml5StreamListener.cpp:91:9
        #13 0x7f6346134f8c in mozilla::net::HttpChannelChild::DoOnDataAvailable(nsIRequest*, nsIInputStream*, unsigned long, unsigned int) /netwerk/protocol/http/HttpChannelChild.cpp:793:29
        #14 0x7f6346132a1d in mozilla::net::HttpChannelChild::OnTransportAndData(nsresult const&, nsresult const&, unsigned long const&, unsigned int const&, nsTSubstring<char> const&) /netwerk/protocol/http/HttpChannelChild.cpp:695:3
        #15 0x7f63465e98b2 in mozilla::net::ChannelEventQueue::FlushQueue() /netwerk/ipc/ChannelEventQueue.cpp:94:12
        #16 0x7f634664450f in CompleteResume /netwerk/ipc/ChannelEventQueue.h:316:5
        #17 0x7f634664450f in mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run() /netwerk/ipc/ChannelEventQueue.cpp:152:17
        #18 0x7f6344fd9689 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1193:16
        #19 0x7f6344fe6cb4 in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:479:10
        #20 0x7f6346bfe7f9 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:330:5
        #21 0x7f6346a24e9a in RunInternal /ipc/chromium/src/base/message_loop.cc:370:10
        #22 0x7f6346a24e9a in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #23 0x7f6346a24e9a in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #24 0x7f6344fd067a in nsThread::ThreadFunc(void*) /xpcom/threads/nsThread.cpp:391:10
        #25 0x7f636b5d3b3f in _pt_root /nsprpub/pr/src/pthreads/ptthread.c:201:5
        #26 0x7f636bb38b42 in start_thread nptl/pthread_create.c:442:8
    
    Thread T4 created by T0 (Isolated Web Co) here:
        #0 0x55f51d4a57ca in pthread_create /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:208:3
        #1 0x7f636b5c22a4 in _PR_CreateThread /nsprpub/pr/src/pthreads/ptthread.c:458:14
        #2 0x7f636b5afe9e in PR_CreateThread /nsprpub/pr/src/pthreads/ptthread.c:533:12
        #3 0x7f6344fd436c in nsThread::Init(nsTSubstring<char> const&) /xpcom/threads/nsThread.cpp:634:18
        #4 0x7f6344fe455e in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, nsIThreadManager::ThreadCreationOptions, nsIThread**) /xpcom/threads/nsThreadManager.cpp:548:12
        #5 0x7f6344ff230c in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, already_AddRefed<nsIRunnable>, nsIThreadManager::ThreadCreationOptions) /xpcom/threads/nsThreadUtils.cpp:175:57
        #6 0x7f634750bdd4 in NS_NewNamedThread<13UL> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:87:10
        #7 0x7f634750bdd4 in nsHtml5Module::InitializeStatics() /parser/html/nsHtml5Module.cpp:63:3
        #8 0x7f6351662bca in nsLayoutStatics::Initialize() /layout/build/nsLayoutStatics.cpp:235:3
        #9 0x7f63516628e9 in nsLayoutModuleInitialize() /layout/build/nsLayoutModule.cpp:104:7
        #10 0x7f6344f66553 in nsComponentManagerImpl::Init() /xpcom/components/nsComponentManager.cpp:371:5
        #11 0x7f6345057a85 in NS_InitXPCOM /xpcom/build/XPCOMInit.cpp:421:51
        #12 0x7f634ef97360 in mozilla::dom::ContentProcess::Init(int, char**) /dom/ipc/ContentProcess.cpp:153:8
        #13 0x7f6356020e27 in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:618:21
        #14 0x55f51d4fb6de in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #15 0x55f51d4fb6de in main /browser/app/nsBrowserApp.cpp:375:18
        #16 0x7f636bacdd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    
    SUMMARY: AddressSanitizer: heap-buffer-overflow /dom/base/nsAttrValueInlines.h:148:68 in GetIntegerValue
    Shadow bytes around the buggy address:
      0x603000013180: 00 00 04 fa fa fa fd fd fd fa fa fa fd fd fd fd
      0x603000013200: fa fa fd fd fd fa fa fa 00 00 06 fa fa fa fd fd
      0x603000013280: fd fd fa fa fd fd fd fa fa fa fd fd fd fd fa fa
      0x603000013300: 00 00 04 fa fa fa fd fd fd fa fa fa fd fd fd fd
      0x603000013380: fa fa fd fd fd fd fa fa fd fd fd fa fa fa 00 00
    =>0x603000013400: 00 00 fa fa 00 00[02]fa fa fa fa fa fa fa fa fa
      0x603000013480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x603000013500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x603000013580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x603000013600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x603000013680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    Shadow byte legend (one shadow byte represents 8 application bytes):
      Addressable:           00
      Partially addressable: 01 02 03 04 05 06 07 
      Heap left redzone:       fa
      Freed heap region:       fd
      Stack left redzone:      f1
      Stack mid redzone:       f2
      Stack right redzone:     f3
      Stack after return:      f5
      Stack use after scope:   f8
      Global redzone:          f9
      Global init order:       f6
      Poisoned by user:        f7
      Container overflow:      fc
      Array cookie:            ac
      Intra object redzone:    bb
      ASan internal:           fe
      Left alloca redzone:     ca
      Right alloca redzone:    cb
    ==3815073==ABORTING
Attached file Testcase — 

    
    

    
  
We've seen a flood of these in the last few hours so I suspect this is a recent regression.


Whiteboard: [bugmon:confirm] → [bugmon:confirm][fuzzblocker]

    
  
Group: core-security → dom-core-security

          status-firefox116:
          --- → affected

    
    

    
  
Regression from bug 1839223?


Flags: needinfo?(emilio)
Keywords: csectype-bounds, 
          
            regression, 
          
            regressionwindow-wanted, 
          
            sec-high

    
    

    
  
[Tracking Requested - why for this release]: sec high, probable regression, with a trivial test case



          tracking-firefox116:
          --- → ?

    
    

    
  
I get this assertion in a debug build, so I think this is more like a type confusion: some kind of variant container is expected to be an int, but it actually has a string (based on the ASan allocation stack).


Assertion failure: Type() == eInteger (wrong type), at /Users/andrewmccreight/mc/dom/base/nsAttrValueInlines.h:146
#01: nsAttrValue::GetIntegerValue() const
#02: mozilla::dom::HTMLTableElement::BuildInheritedAttributes()
#03: mozilla::dom::HTMLTableElement::BindToTree(mozilla::dom::BindContext&, nsINode&)
#04: nsINode::InsertChildBefore(nsIContent*, nsIContent*, bool, mozilla::ErrorResult&)




    
    

    
  
The prior code, MapInheritedTableAttributesIntoRule, did an explicit check that value->Type() == nsAttrValue::eInteger is true after getting the value of the cellpadding attribute.



    
  
Assignee: nobody → continuation

    
  
OS: Linux → All
Hardware: x86_64 → All

    
    

    
  
This is also showing up in Nightly: bp-47e83a97-2bc2-4957-a165-474630230623


All null crashes for what that is worth.



    
  
Crash Signature: [@ nsAttrValue::GetIntegerValue ]

    
    

    
  
Here is the check in the old code.



    
    

    
  
Verified bug as reproducible on mozilla-central 20230623092529-750c24176cc2.

The bug appears to have been introduced in the following build range:




Start: 19c777f3b143ca86c79eb065fef63b104200fe97 (20230622134915)

End: b9e51e69d0451a93822b61cc15d72666124f02dd (20230622144102)

Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=19c777f3b143ca86c79eb065fef63b104200fe97&tochange=b9e51e69d0451a93822b61cc15d72666124f02dd




Whiteboard: [bugmon:confirm][fuzzblocker] → [bugmon:bisected,confirmed][fuzzblocker]

    
  
Flags: needinfo?(emilio)

    
    

    
  
Add back type check to HTMLTableElement::BuildInheritedAttributes(). r=nika

https://hg.mozilla.org/integration/autoland/rev/897b88c7e2ed402ae0bd51a5e0174c7637699fd7

https://hg.mozilla.org/mozilla-central/rev/897b88c7e2ed


Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 1 year ago

          status-firefox116:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch
Duplicate of this bug: 1840112

    
    

    
  
Verified bug as fixed on rev mozilla-central 20230625195949-a529a3830f68.


Status: RESOLVED → VERIFIED

    
    

    
  
This is more like a type confusion on a union type, so maybe wildptr is better than bounds?


Keywords: csectype-bounds → csectype-wildptr

    
    

    
  
Set release status flags based on info from the regressing bug 1839223



          status-firefox-esr115:
          --- → unaffected
Group: core-security-release
Keywords: bugmon

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: