Bug 51055 – NULL deref in WebCore::HTMLEntitySearch::advance

• Component: DOM
• Status: RESOLVED FIXED

[Thomas Sepez]

As initially reported in http://code.google.com/p/chromium/issues/detail?id=66098 by

[Thomas Sepez]

Reported by aohelin, Dec 09 (5 days ago)

VULNERABILITY DETAILS
Opening the attached file causes a renderer segmentation fault. The crash looks like a null, behaves like a null and quacks like a null, so it is probably harmless. On the other hand it is in a core component and affects all versions of Chrome I tested, so reporting conservatively as a security issue.

I did not find a way to change the crash address or location.

VERSION
Chrome Version: Chromium 8.0.552.215 (Developer Build 67652) Ubuntu 10.10 (beta), Chrome 8.0.552.215 (Official Build 67652) (stable)
Operating System: Ubuntu 10.10, 32- and 64-bit

REPRODUCTION CASE
Open the attached parse.svg, or data:image/svg+xml,<!DOCTYPE foo PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" ""> <foo foo="&:;

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: tab
Crash State: 
Program received signal SIGSEGV, Segmentation fault.
WebCore::HTMLEntitySearch::advance (this=0xbfffd8cc, nextCharacter=58)
    at third_party/WebKit/WebCore/html/parser/HTMLEntitySearch.cpp:124
124     third_party/WebKit/WebCore/html/parser/HTMLEntitySearch.cpp: No such file or directory.
        in third_party/WebKit/WebCore/html/parser/HTMLEntitySearch.cpp
(gdb) bt 10
#0  WebCore::HTMLEntitySearch::advance (this=0xbfffd8cc, nextCharacter=58)
    at third_party/WebKit/WebCore/html/parser/HTMLEntitySearch.cpp:124
#1  0xb674e3f3 in WebCore::decodeNamedEntity (name=0xb8241447 ":")
    at third_party/WebKit/WebCore/html/parser/HTMLEntityParser.cpp:257
#2  0xb6af2a27 in getXHTMLEntity (closure=0xb8288400, name=0xb8241447 ":")
    at third_party/WebKit/WebCore/dom/XMLDocumentParserLibxml2.cpp:1209
#3  WebCore::getEntityHandler (closure=0xb8288400, name=0xb8241447 ":")
    at third_party/WebKit/WebCore/dom/XMLDocumentParserLibxml2.cpp:1241
#4  0xb62f61fc in xmlParseEntityRef (ctxt=0xb8288400)
    at third_party/libxml/src/parser.c:7164
#5  0xb62fbd85 in xmlParseAttValueComplex (ctxt=0xb8288400, 
    len=<value optimized out>, alloc=0xbfffdaf0, normalize=0)
    at third_party/libxml/src/parser.c:3701
#6  xmlParseAttValueInternal (ctxt=0xb8288400, len=<value optimized out>, 
    alloc=0xbfffdaf0, normalize=0) at third_party/libxml/src/parser.c:8578
#7  0xb62fc9ac in xmlParseAttribute2 (ctxt=0xb8288400, 
    pref=<value optimized out>, URI=0xbfffdb9c, tlen=0xbfffdbb0)
    at third_party/libxml/src/parser.c:8634
#8  xmlParseStartTag2 (ctxt=0xb8288400, pref=<value optimized out>, 
    URI=0xbfffdb9c, tlen=0xbfffdbb0) at third_party/libxml/src/parser.c:8792
#9  0xb63031ad in xmlParseTryOrFinish (ctxt=0xb8288400, 
    terminate=<value optimized out>) at third_party/libxml/src/parser.c:10843
(More stack frames follow...)
(gdb) bt 2 full
#0  WebCore::HTMLEntitySearch::advance (this=0xbfffd8cc, nextCharacter=58)
    at third_party/WebKit/WebCore/html/parser/HTMLEntitySearch.cpp:124
No locals.
#1  0xb674e3f3 in WebCore::decodeNamedEntity (name=0xb8241447 ":")
    at third_party/WebKit/WebCore/html/parser/HTMLEntityParser.cpp:257
        search = {m_currentLength = 1, m_currentValue = 0, 
          m_mostRecentMatch = 0x0, m_first = 0x0, m_last = 0x0}
        entityValue = <value optimized out>
(More stack frames follow...)
(gdb) disas $eip-32, $eip+8
Dump of assembler code from 0xb674f35a to 0xb674f382:
   0xb674f35a <WebCore::HTMLEntitySearch::advance(UChar)+42>:   mov    %edi,(%esp)
   0xb674f35d <WebCore::HTMLEntitySearch::advance(UChar)+45>:   call   0xb6868a80 <WebCore::HTMLEntityTable::firstEntryStartingWith(UChar)>
   0xb674f362 <WebCore::HTMLEntitySearch::advance(UChar)+50>:   mov    %eax,0xc(%esi)
   0xb674f365 <WebCore::HTMLEntitySearch::advance(UChar)+53>:   mov    %edi,(%esp)
   0xb674f368 <WebCore::HTMLEntitySearch::advance(UChar)+56>:   call   0xb6868ad0 <WebCore::HTMLEntityTable::lastEntryStartingWith(UChar)>
   0xb674f36d <WebCore::HTMLEntitySearch::advance(UChar)+61>:   mov    (%esi),%edx
   0xb674f36f <WebCore::HTMLEntitySearch::advance(UChar)+63>:   mov    %eax,0x10(%esi)
   0xb674f372 <WebCore::HTMLEntitySearch::advance(UChar)+66>:   lea    0x1(%edx),%eax
   0xb674f375 <WebCore::HTMLEntitySearch::advance(UChar)+69>:   mov    0xc(%esi),%edx
   0xb674f378 <WebCore::HTMLEntitySearch::advance(UChar)+72>:   mov    %eax,(%esi)
=> 0xb674f37a <WebCore::HTMLEntitySearch::advance(UChar)+74>:   cmp    0x4(%edx),%eax
   0xb674f37d <WebCore::HTMLEntitySearch::advance(UChar)+77>:   je     0xb674f3d3 <WebCore::HTMLEntitySearch::advance(UChar)+163>
   0xb674f37f <WebCore::HTMLEntitySearch::advance(UChar)+79>:   movl   $0x0,0x4(%esi)
(gdb) info registers
eax            0x1      1
ecx            0xffffffd9       -39
edx            0x0      0
ebx            0xb80d1d14       -1207100140
esp            0xbfffd870       0xbfffd870
ebp            0xbfffd8a8       0xbfffd8a8
esi            0xbfffd8cc       -1073751860
edi            0x3a     58
eip            0xb674f37a       0xb674f37a <WebCore::HTMLEntitySearch::advance(UChar)+74>
eflags         0x10286  [ PF SF IF RF ]
cs             0x73     115
ss             0x7b     123
ds             0x7b     123
es             0x7b     123
fs             0x0      0
gs             0x33     51

[Thomas Sepez]

This is a straight null parser de-ref.  It can also be triggered by loading an XML file of the form:


<!DOCTYPE foo PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" ""> 
<foo foo="&:;


The HTMLEntityTable::firstEntryStartingWith() method called by HTMLEntitySearch::Advance() may well return null when passed a non-alpha character.

The code above this allows an entity name to contain a number of (legit) non-alpha characters.

[Thomas Sepez]

Created attachment 76575 [details]
Propose Patch

[Alexey Proskuryakov]

This patch isn't marked for review, please do mark it (click on Details link).

[Alexey Proskuryakov]

Comment on attachment 76575 [details]
Propose Patch

To request review, mark r?. Please see <http://webkit.org/coding/contributing.html>.

[WebKit Review Bot]

Attachment 76575 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/parser/resources/xml-colon-entity.xml', u'LayoutTests/fast/parser/xml-colon-entity-expected.txt', u'LayoutTests/fast/parser/xml-colon-entity.html', u'WebCore/ChangeLog', u'WebCore/html/parser/HTMLEntitySearch.cpp']" exit_code: 1
WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thomas Sepez]

Created attachment 76775 [details]
Proposed patch without tabs in ChangeLogs

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 76775 [details]:

http/tests/navigation/target-frame-from-window.html bug 51098 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 76775 [details]
Proposed patch without tabs in ChangeLogs

Clearing flags on attachment: 76775

Committed r74321: <http://trac.webkit.org/changeset/74321>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.