496779 - Potential buffer overflow in DTD parser

Status: RESOLVED WORKSFORME

(Core :: DOM: HTML Parser, defect)

Description

[Daniel Franke]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090403 Shiretoko/3.1b3
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090403 Shiretoko/3.1b3

nsEntryStack::EnsureCapacityFor() from parser/htmlparser/src/nsDTDUtils.cpp returns silently if memory allocation fails while growing the entry stack.  This could potentially allow a malicious DTD to cause a buffer overflow when a new entry is pushed onto the stack.

Reproducible: Didn't try

Steps to Reproduce:
N/A



The build identifier above is what I happen to be running, but the code is the same in trunk and a comment dates it at April 22, 1999, so it is likely to be present in every Mozilla version.

Comment 1

[Daniel Veditz [:dveditz]]

A few months newer than that comment, but yeah, this is old
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/htmlparser/src/nsDTDUtils.cpp&rev=3.158&mark=126#126

The good news is that as long as new throws on OOM this will cause a non-exploitable crash before the calling code can overflow the heap buffer. new hasn't always thrown (iirc VC 6 doesn't and we're still using that for Thunderbird 2 on Windows) and some non-Mozilla users of this code might still compile it that way -- we should fix this.

Another mitigating factor is that the data that will overwrite is pretty restricted -- a tag enum (attacker can control the node type), nulls, and pointers. But we've seen a few rare hackers pull off some incredible stuff so never say never.

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: one way to fix it

This would be one way to make this safer. Not sure what the effects of silently dropping items would be, but if we're OOM the results aren't going to be pretty in any case. And in practice we'll have already died so maybe we only want this on the 1.8 branch for Thunderbird.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 382093 [details] [diff] [review]
one way to fix it

Sure.

Comment 4

[Daniel Franke]

I just learned about Mozilla's security bounty program.  Does this report qualify?

Comment 5

[Daniel Veditz [:dveditz]]

It would qualify for the bounty only if you can demonstrate a buffer overwrite in code as shipped by Mozilla. As far as we can see this crashes in an unexploitable way before the overwrite would happen (a "denial of service" (DoS) bug). DoS crashes are not covered by the Bug Bounty program.
http://www.mozilla.org/security/bug-bounty-faq.html#dos-bugs

Comment 7

[Andrei Purice]

Hey Daniel
Does this issue still occur for you or can it be closed?

Comment 8

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-08-31)]

This was fixed by removing this code in bug 903912.