Bug 41107

Summary: REGRESSION (r61619): Memory corruption in open-source ICO decoder
Product: WebKit Reporter: Peter Kasting <pkasting>
Component: ImagesAssignee: Peter Kasting <pkasting>
Status: RESOLVED FIXED    
Severity: Normal CC: 465782708, abarth, carlossantoing, eric, Grace_Cooper406, mrobinson, webkit.review.bot, yunretta1221547
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: All   
URL: http://www.opennet.ru/opennews/art.shtml
Attachments:
Description Flags
patch v1 abarth: review+

Description Peter Kasting 2010-06-23 15:16:08 PDT
REGRESSION (r61619): Memory corruption in open-source ICO decoder
Comment 1 Peter Kasting 2010-06-23 15:19:18 PDT
BMPImageReader.cpp erroneously accesses |m_parent| when setSize() fails.  setSize() has already called setFailed(), which has deleted |this|, thus we shouldn't access |m_parent| (and don't need to).
Comment 2 Peter Kasting 2010-06-23 16:12:16 PDT
Created attachment 59572 [details]
patch v1

Fixes the corruption and one other technically-wrong place I noticed.

This adds a regression .ico to an existing LayoutTest, unfortunately I can't actually update expected results at the moment, so I'm going to need those from somewhere.
Comment 3 Adam Barth 2010-06-24 14:33:53 PDT
Comment on attachment 59572 [details]
patch v1

ok
Comment 4 Peter Kasting 2010-06-24 15:02:21 PDT
Fixed in r61788.  I'll land the updated test expectations once the bots have them.
Comment 5 WebKit Review Bot 2010-06-24 17:26:29 PDT
http://trac.webkit.org/changeset/61800 might have broken SnowLeopard Intel Release (Tests)
Comment 7 Adam Barth 2010-06-24 22:26:43 PDT
Hopefully fixed in http://trac.webkit.org/changeset/61821
Comment 8 Peter Kasting 2010-06-25 12:02:31 PDT
The reason I didn't update GTK is because it matches Chromium rather than Safari and it wasn't clear to me that those two text dumps would be the same.  (And I couldn't get a Chromium dump at the time.)

I'm not lazy!  I was just gone.  I got two other people to help take care of this before I had to go.