Bug 135040 - inserting an SVGLength that is already in an SVGAnimatedLengthList::animVal into another list causes a crash
Summary: inserting an SVGLength that is already in an SVGAnimatedLengthList::animVal i...
Status: RESOLVED DUPLICATE of bug 191237
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://mcc.id.au/temp/list4.html
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-17 23:11 PDT by Cameron McCormack (:heycam)
Modified: 2019-05-06 12:40 PDT (History)
5 users (show)

See Also:

Attachments
local copy of test case (1.28 KB, text/html)
2014-07-19 23:31 PDT, Darin Adler 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron McCormack (:heycam) 2014-07-17 23:11:24 PDT 
STR:

1. Load the URL.
2. Crash.

Attempting to insert an SVGLength that came from an SVGAnimatedLengthList's animVal into another SVGLengthList will cause a crash.  I haven't looked to see if the crash is serious.  Tested with Safari 7.0.5 (9537.77.4).
Comment 1 Darin Adler 2014-07-19 22:32:54 PDT 
This crash is a null dereference; I doubt there is any security implication, and I suspect this is mis-categorized as a security bug.

The bug seems to be in code in SVGListPropertyTearOff::processIncomingListItemWrapper, which calls SVGAnimatedListPropertyTearOff::findItem, which can’t handle an m_baseVal of nullptr. If I knew this code better, I might suggest fixing this by calling baseVal() inside findItem and in removeItemFromList rather than accessing m_baseVal directly.
Comment 2 Dirk Schulze 2014-07-19 23:23:34 PDT 
(In reply to comment #1)
> This crash is a null dereference; I doubt there is any security implication, and I suspect this is mis-categorized as a security bug.
> 
> The bug seems to be in code in SVGListPropertyTearOff::processIncomingListItemWrapper, which calls SVGAnimatedListPropertyTearOff::findItem, which can’t handle an m_baseVal of nullptr. If I knew this code better, I might suggest fixing this by calling baseVal() inside findItem and in removeItemFromList rather than accessing m_baseVal directly.

I asked Cameron to open it as security bug since I couldn't test it myself yet. If you have evidence that it is a normal crash, please re-categorize it.
Comment 3 Darin Adler 2014-07-19 23:30:21 PDT 
OK, done.
Comment 4 Darin Adler 2014-07-19 23:31:03 PDT 
Created attachment 235179 [details]
local copy of test case
Comment 5 Darin Adler 2014-07-19 23:31:29 PDT 
Crashes after trying to access a data member of m_baseVal, which is nullptr.
Comment 6 Zoltan Horvath 2014-12-09 16:00:17 PST 
Blink doesn't have the issue, probably because of some refactoring on this code. Merge candidate tracked in: https://bugs.webkit.org/show_bug.cgi?id=125888
Comment 7 Said Abou-Hallawa 2019-05-06 12:40:15 PDT 
After https://bugs.webkit.org/show_bug.cgi?id=191237 is fixed and when opening the attached test case in WebKit, I get the following output:

appending an SVG item that is in an animVal list of another element:
  - animVal list item remained the same object
  - an item was appended
  - the appended item was a copy of the animVal list item

This is complies with SVG 2 specs: https://www.w3.org/TR/SVG2/types.html#__svg__SVGNameList__appendItem. Also this is the behavior of FireFox.

*** This bug has been marked as a duplicate of bug 191237 ***