Bug 40837 – Chrome/Safari Crashes on SVG Image

• Component: SVG
• Status: RESOLVED WORKSFORME

[W. James MacLean]

Created attachment 59112 [details]
Unix_history-simple.svg

The following SVG causes recent nightly builds (Safari/WebKit [Mac], Chromium [Mac,Linux]) to crash.

http://upload.wikimedia.org/wikipedia/commons/7/77/Unix_history-simple.svg

[W. James MacLean]

Created attachment 59127 [details]
Proposed patch

This patch is marked for review to discuss the approach to fixing the bug, and does not (yet) include a simplified test and changelog comments.

The bug appears to occur in

SVGRootInlineBox::layoutInlineBoxes(InlineFlowBox* start, Vector<SVGChar>::iterator& it, int& lowX, int& highX, int& lowY, int& highY)

when a non-text InlineFlowBox is encountered that has no children. In this case the default values

int minX = INT_MAX;
int minY = INT_MAX;
int maxX = INT_MIN;
int maxY = INT_MIN;

are retained and lead to the current InlineBox being assigned unrealistically large (x,y) coordinates.

These coordinates then lead to unrealistically large bounding rects, causing the platform-level renderers to fail.

The proposed patch exits, leaving the default size for the InlineBox, which appears to work (performance may be slow, but this appears to be a separate issue with the platform-level filters).

[W. James MacLean]

Update: I just re-sync'd, and since yesterday the file SVGRootInlineBox.cpp has been completely re-architected. Based on my fresh build of Safari though, it looks like the bug is still there, although opening this SVG will really thrash your machine before it takes Safari down ...

[Nikolas Zimmermann]

(In reply to comment #2)
> Update: I just re-sync'd, and since yesterday the file SVGRootInlineBox.cpp has been completely re-architected. Based on my fresh build of Safari though, it looks like the bug is still there, although opening this SVG will really thrash your machine before it takes Safari down ...

Hm, it works fine for me w/o any performance and/or crash issues? Using Safari4 on 10.5.8.

[Dirk Schulze]

(In reply to comment #3)
> (In reply to comment #2)
> > Update: I just re-sync'd, and since yesterday the file SVGRootInlineBox.cpp has been completely re-architected. Based on my fresh build of Safari though, it looks like the bug is still there, although opening this SVG will really thrash your machine before it takes Safari down ...
> 
> Hm, it works fine for me w/o any performance and/or crash issues? Using Safari4 on 10.5.8.

Can't reproduce it either. Neither in WebKitGtk nor in Chromium-linux.

[Adam Barth]

Comment on attachment 59127 [details]
Proposed patch

Thanks for the patch, but we'll need a ChangeLog describing why you're making this change (and linking to this bug) as well as a test to make sure we don't screw this up in the future.  You can find more information about submitting patch here:

http://webkit.org/coding/contributing.html

[Abhishek Arya]

W. James, you might want to use http://upload.wikimedia.org/wikipedia/commons/d/d9/Unix_history-simple.en.svg (from security bug https://bugs.webkit.org/show_bug.cgi?id=39253) as your testcase since it reliably crashes every time i use it :). Dirk is already taking a look at 39253. Also have a quick question, do you see any security issue like memory corruption, etc inside debugger. when we were testing bug 39253, we could also reproduce a null ptr exception arising from a imagebuffer can't being allocated from large size. If that is the case, we can unhide/remove security flags from 39253 as well. Otherwise, we need to mark this bug as security and hide it.

[W. James MacLean]

(In reply to comment #6)
> W. James, you might want to use http://upload.wikimedia.org/wikipedia/commons/d/d9/Unix_history-simple.en.svg 

Thanks. This was the image I intended to use, but I must have gotten the wrong link.

I've been find the latest pull from the repository doesn't seem to have the crashing problem anymore, although there is still a large performance hit (at least on Chrome-Linux). I'll compare the two images to see if there's a difference that points us towards the problem.

I guess there is the possibility of a null pointer exception in release builds - the debug builds die on an assert (at least in older versions of the code that had the problem).

[W. James MacLean]

Created attachment 59516 [details]
SVG that (may) crash and/or cause performance issues.

Revised SVG image that (may) cause crash/performance issues.

[Simon Fraser (smfr)]

Seems fine in a recent nightly.

[Rob Buis]

Checked again, works fine nowadays.