320375 - CVE-2006-0298 Buffer overrun in nsExpatDriver::ParseBuffer [@ nsExpatDriver::ParseBuffer]

Status: RESOLVED FIXED

(Core :: XML, defect, P1)

Description

[Peter Van der Beken [:peterv]]

Jst discovered this while helping me debug my patch for bug 274777 (I'd already fixed it without realizing it in my patch for bug 22942).

See http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/htmlparser/src/nsExpatDriver.cpp&rev=3.86&mark=879#871 It should read |startOffset = aLength / sizeof(PRUnichar);|.

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 2

[Peter Van der Beken [:peterv]]

There's quite a bit of crashes in Talkback that seem caused by this: http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsExpatDriver::ParseBuffer&vendor=MozillaOrg&product=Firefox15&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid It's number 37 in the topcrashers list for 1.5. The crash usually happens when we access the buffer, probably with an index that's too big: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/parser/htmlparser/src/nsExpatDriver.cpp&mark=892&rev=MOZILLA_1_8_BRANCH#872

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 205954 [details] [diff] [review]
v1

sr=jst

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 205954 [details] [diff] [review]
v1

r=mrbkap

Comment 5

[Peter Van der Beken [:peterv]]

Checked in on trunk.

Comment 6

[Mike Schroepfer]

Can we bake a few more takes and then we'll consider.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 205954 [details] [diff] [review]
v1

a=dveditz
Please add the fixed1.8.0.1 and fixed1.8.1 when checked in to the branches

Comment 8

[Marcia Knous [:marcia]]

Would like to verify this on the branch, but without a testcase or STR it is tough. Can someone help here?

Comment 9

[Marcia Knous [:marcia]]

In order to certify the 1.5.0.1 release, I have been asked to verify this bug. Can I please get some help getting a testcase or STR to verify?  Thanks.

Comment 10

[Peter Van der Beken [:peterv]]

I'll try to come up with a testcase but it's not trivial. This bug was discovered through code inspection.
Note that there are a bunch of talkbacks (see comment 2), but I never got the crash myself.

Comment 11

[Marcia Knous [:marcia]]

Peter: I don't want to cause excessive work, so if it will take too much time we can have someone inspect the code to verify.

(In reply to comment #10)
> I'll try to come up with a testcase but it's not trivial. This bug was
> discovered through code inspection.
> Note that there are a bunch of talkbacks (see comment 2), but I never got the
> crash myself.
> 

Comment 12

[Peter Van der Beken [:peterv]]

The patch that caused this wasn't checked in on the 1.7.x branch.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

nsExpatDriver::ParseBuffer is still showing up as a topcrash for Fx 1.5.0.1.