330998 - Crash [@ nsLayoutUtils::GetFloatFromPlaceholder] involving tables, forms, and float

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Jesse Ruderman]

Bob Clary found this bug; I reduced it (with help from Lithium).

Filing as security-sensitive because the variation in behavior (see below) and partial stacks in Win2003 Talkback make me worry this is a memory corruption bug.

* Windows 2003: TB16554961Z (partial stack)

* Windows XP: Instead of crashing, it goes into a screwed-up state (where toolbar buttons don't respond visually to clicks unless the mouse moves during the click, and firefox.exe doesn't exit when I close the last window).

* Mac: TB16555048M (full stack)

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Jesse Ruderman]

Attachment: stack trace (mac debug / gdb)

Comment 3

[Bernd]

we hit 
###!!! ASSERTION: Null out-of-flow for placeholder?: 'outOfFlow', file d:\moz_src\mozilla\layout\generic\nsPlaceholderFrame.h, line 121

Comment 4

[Bernd]

sounds like bug 285727

Comment 5

[Martijn Wargers (dead)]

Perhaps bug 282173 will fix it?

Comment 6

[Jesse Ruderman]

Still crashes in Mac trunk 2006-04-10.  So not fixed by BuildFloatList removal.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Yeah... this is basically bug 285727....

At least it's just a null-dereference (well, virtual function call on NULL, which isn't quite the same thing).

Comment 8

[Boris Zbarsky [:bzbarsky]]

I take that back.  GetStyleData is non-virtual when called inside layout.  So this is not really a big security issue, imo.

Comment 9

[Jesse Ruderman]

I think even a virtual function call on NULL would not be a security hole, as long as the OS denies access to the entire first page of memory and the object isn't too complicated.

Comment 10

[Bernd]

fixed by the patch for bug 285727

Comment 11

[Daniel Veditz [:dveditz]]

Sounds like this isn't a security problem after all. We should be able to remove the confidential flag, right?

Comment 12

[Bob Clary [:bc] (inactive)]

(In reply to comment #11)
> Sounds like this isn't a security problem after all. We should be able to
> remove the confidential flag, right?
> 

NO! this and all fuzz testing bugs should be confidential until we have a better handle on things.

Comment 13

[Jesse Ruderman]

There wasn't much mention of fuzz-testing in this bug until comment 12 ;)  I try to file these bugs with reduced testcases and without mentioning how I found them.

Comment 14

[Bob Clary [:bc] (inactive)]

I was going on the blocking bug 306663

Comment 15

[Daniel Veditz [:dveditz]]

Clearing the security flag. It's no secret that we test Firefox, we just don't want to hand over juicy testcases.

Comment 16

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060829 Firefox/1.5.0.7

Firefox1.5.0.6 still crashes.

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060830 BonEcho/2.0b2

Comment 17

[Jesse Ruderman]

Crashtest checked in.