Bug 16888 – -webkit-border-image crash/invalid free

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Michael Goddard]

There's an error in the CSSParser when parsing the width components of -webkit-border-image.  A pointer to the middle of an array is stored in an OwnPtr and gets freed.  Can cause crashes/memory corruption.

Testcase/patch to be attached.

[Michael Goddard]

Created attachment 18465 [details]
Testcase and patch

[Michael Goddard]

The test case style for the <div> could probably be removed.

[Sam Weinig]

Comment on attachment 18465 [details]
Testcase and patch

There are some tabs in the patch and I don't understand the ChangeLog entirely.  Where is the OwnPtr that you are storing to?

[Dave Hyatt]

Comment on attachment 18465 [details]
Testcase and patch

I don't quite understand this patch either.  I don't see why a Value would have to be copied.

[Michael Goddard]

Created attachment 18485 [details]
Fix memory corruption - just store Values as member vars, don't allocate them

In the original code, m_borderTop etc were OwnPtr<Value>s, and so we needed to give them a valid pointer (hence the allocation in the previous patch, rather than the middle of an array).  Since the BorderImageParseContext is stack allocated anyway, just make it slightly larger to hold actual Values and copy them in.  This needs an extra variable to track which Values are valid.

[Michael Goddard]

Another option would be to just store pointers to the middle of the callers ValueList, since hopefully the lifetime is longer than the BorderImageParseContext.

[Michael Goddard]

Created attachment 18516 [details]
Use naked pointers rather than OwnPtrs since the source pointers are in the middle of an array

Simpler than previous patch and avoids any extra allocations.

[David Kilzer (:ddkilzer)]

Loading the test case, the following is printed to the console for times per page load on a debug build of WebKit r29623:

Safari(28864,0xa000ed88) malloc: ***  Deallocation of a pointer not malloced: 0xc199030; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug

[David Kilzer (:ddkilzer)]

<rdar://problem/5696235>

[Darin Adler]

Comment on attachment 18516 [details]
Use naked pointers rather than OwnPtrs since the source pointers are in the middle of an array

r=me

[Simon Hausmann]

Landed in r29764