1089438 - Bad-casting from PRCListStr to nsSHistory

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[lifeasageek]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.120 Safari/537.36

Steps to reproduce:





Actual results:

Hi Firefox!

Please refer bug1074280 for this report.

// From docshell/shistory/src/nsSHistory.cpp:1145

    transactions.AppendElements(shTransactions);
    shist = static_cast<nsSHistory*>(PR_NEXT_LINK(shist));

At the end of the loop, PR_NEXT_LINK(shist) points to the static global variable, "static PRCList gSHistoryList". Thus, the object pointed by PR_NEXT_LINK(shist) is not a sub-object of nsSHistory, and triggers undefined-behavior.

---------------------------------------------

/home/blee/project/firefox/firefox/docshell/shistory/src/nsSHistory.cpp:1145:13: Casting from 'PRCListStr' to 'nsSHistory'
         Pointer         0x7f66feee6be0                                                                                                                                      
         Alloc base      0x000000000000
         User base       0x000000000000
         Offset          0x7f66feee6be0
         TypeTable       0x000000000000
    #0 0x404e19 in __csan_handle_cast /home/blee/project/cast_sanitizer/llvm/projects/compiler-rt/lib/csan/csan_common.cc:353
    #1 0x7f66fbc5e907 in GloballyEvictContentViewers /home/blee/project/firefox/firefox/docshell/shistory/src/nsSHistory.cpp:1145 (discriminator 4)
    #2 0x7f66fbc6073e in EvictOutOfRangeContentViewers /home/blee/project/firefox/firefox/docshell/shistory/src/nsSHistory.cpp:806
    #3 0x7f66fb8674ac in Show /home/blee/project/firefox/firefox/layout/base/nsDocumentViewer.cpp:2017
    #4 0x7f66fb90a77b in EnsureVisible /home/blee/project/firefox/firefox/layout/base/nsPresContext.cpp:2015
    #5 0x7f66fb875d17 in UnsuppressAndInvalidate /home/blee/project/firefox/firefox/layout/base/nsPresShell.cpp:3928 (discriminator 2)
    #6 0x7f66fb864f40 in LoadComplete /home/blee/project/firefox/firefox/layout/base/nsDocumentViewer.cpp:1043

------------------------------
Name    struct PRCListStr
Size    16
NumVar  2
NumBase 0
------------------------------
   0 | struct PRCListStr                                                                                                                                                     
   0 |   PRCList * next
   8 |   PRCList * prev
     | [sizeof=16, dsize=16, align=8
     |  nvsize=16, nvalign=8]

------------------------------
Name    class nsSHistory
Size    128
NumVar  8
NumBase 4
        0       class nsISHistory ['(primary base)']
        8       struct PRCListStr ['(base)']
	24      class nsISHistoryInternal ['(base)']
	32      class nsIWebNavigation ['(base)']

------------------------------
   0 | class nsSHistory
   0 |   class nsISHistory (primary base)
   0 |     class nsISupports (primary base)
   0 |       (nsISupports vtable pointer)
   8 |   struct PRCListStr (base)
   8 |     PRCList * next
  16 |     PRCList * prev
  24 |   class nsISHistoryInternal (base)
  24 |     class nsISupports (primary base)
  24 |       (nsISupports vtable pointer)
  32 |   class nsIWebNavigation (base)
...

Comment 1

[Benjamin Smedberg]

hg blame has bug 683777 r=smaug, so starting with NEEDINFO there.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

The code is actually ancient, coming from bug 292965.
And as far as I see we just end up using that pointer in a pointer comparison so nothing bad should happen.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v1

I guess this should silence the warning.

Comment 4

[Andrew McCreight [:mccr8]]

> Please refer bug1074280 for this report.

Thanks for the report.  In general, it would be good to just copy and paste your brief explanation of what is happening when you file these bugs, because somebody who has access to one bug may not have access to the other.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfa520bee5fc

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cfa520bee5fc

Comment 7

[Al Billings [:abillings - ex-MoCo]]

Can we get a security rating for this issue? If this is serious, we need to backport this to Aurora, Beta, and ESR31.

This shouldn't have landed without one (and sec-approval+ if it is high or critical) unless it is trunk only. Since this seems to go back to 2011, I assume everything is affected. See https://wiki.mozilla.org/Security/Bug_Approval_Process

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I don't see anything critical/high here. We would have either crashed (in a bad way) or ended up to a
endless loop if something had gone wrong. The code is ancient and we haven't seen issues with it.