RESOLVED FIXED 39253
NULL deref in SVG FilterEffect::getEffectContext()
https://bugs.webkit.org/show_bug.cgi?id=39253
Summary NULL deref in SVG FilterEffect::getEffectContext()
Justin Schuh
Reported 2010-05-17 17:45:47 PDT
Originally reported by delcypher to http://crbug.com/43529 This looks like a simple NULL deref where FilterEffect::getEffectContext() should be checking the return value of ImageBuffer::create(). However, there does seem to be an issue with the filter processing, because the results seem well outside the range of where they should be. The file is a Wikipedia graphic; I've attached a copy in case the version at the server changes.
Attachments
Crashing SVG (77.36 KB, image/svg+xml)
2010-05-17 17:46 PDT, Justin Schuh
no flags
Reduced testcase (1.62 KB, image/svg+xml)
2010-06-24 02:15 PDT, Nikolas Zimmermann
no flags
Justin Schuh
Comment 1 2010-05-17 17:46:28 PDT
Created attachment 56296 [details] Crashing SVG
David Kilzer (:ddkilzer)
Comment 2 2010-05-17 17:49:09 PDT
Abhishek Arya
Comment 3 2010-06-21 21:07:56 PDT
+ccing the SVG folks @krit, zimmermann
Dirk Schulze
Comment 4 2010-06-21 22:58:17 PDT
(In reply to comment #3) > +ccing the SVG folks @krit, zimmermann We had a crash report about the same graphic but without filters at bug 40837. Strange. Indeed there is a check for ImageBuffer missing, but if the ImageBuffer is not created, the filter effect won't be applied to the shape. There is a bug in <use>, it doesn't give back the correct size, so that the image size is often as big as the viewport size and in the case of the graphic it would be huge. Maybe this can influence the image creation somehow. I'll check it.
Justin Schuh
Comment 5 2010-06-23 07:20:37 PDT
+wjmaclean@chromium.org from bug 40837 krit - The image definitely changed at the server. Last time I looked at this I was almost certain it was a simple NULL deref (so, not a security issue). However, I hadn't worked out what was causing the extremely large filter values, so I filed as security just to be safe. It sounds like I was hitting the use bug you're describing. If that's the case, and it's just a simple NULL deref / resource consumption thing, then I think it's safe to drop the security flags.
W. James MacLean
Comment 6 2010-06-23 09:10:55 PDT
(In reply to comment #5) The cause of the extremely large bitmap size appears to have been in SVGRootInlineBox::layoutInlineBoxes(), where a non-text inline flow box *without* children generates extremely large offset values based on INTMAX. This code seems to have been completely re-architected recently, with the function I mention above having completely disappeared. The new code *doesn't seem* to have the same problem, although I have not determined that with great certainty. There 8do* seem to be performance issues though ... on my up-to-date Chrome-Linux build it takes several *minutes* to render the image.
Nikolas Zimmermann
Comment 7 2010-06-24 02:15:07 PDT
Created attachment 59625 [details] Reduced testcase Attaching a reduced testcase. Very simple. Two problems: - <use filter=".."> does not seem to have any effect in this testcase. - <rect filter=".."> alone is already slow. If you remove all filters the original testcase loads fast in under a second here, and with just _one_ filter present, it takes much longer. Dirk, can you please have a look? This looks important to me. Note, <text> is no problem at all, and I stripped it completely from the reduced testcase, so we can focus on the real problem. I suspect it has sth. to do with large intermediate buffers.
Dirk Schulze
Comment 8 2010-06-24 03:00:36 PDT
(In reply to comment #7) > Created an attachment (id=59625) [details] > Reduced testcase > > Attaching a reduced testcase. Very simple. Two problems: > - <use filter=".."> does not seem to have any effect in this testcase. > - <rect filter=".."> alone is already slow. > If you remove all filters the original testcase loads fast in under a second here, and with just _one_ filter present, it takes much longer. > > Dirk, can you please have a look? This looks important to me. > Note, <text> is no problem at all, and I stripped it completely from the reduced testcase, so we can focus on the real problem. I suspect it has sth. to do with large intermediate buffers. Investigated a bit more on it. And found the reasons for the problem: <filter id="Shadow" filterUnits="userSpaceOnUse"> No x,y,width,or height are specified. So we take -10%,-10%,120%,120% of the viewport size. This makes the intermediate buffer sizes (2156.400146, 1371.600098). This is not realy a bug in WebKit, beside that the ImageBuffers seems not created fast enough. The author shouldn't use userSpaceOnUse here. The feature plan for SVG Filter was to calculate the smallest neccessary area, that we will use. But this takes some time. I propose to add a return 0, if the ImageBuffer didn't get created for the moment. It's difficult to write a test case for this problem, because the test should be fast enough for DRT. Niko, I didn't blame text for the issue and you're right, that <use> wasn't the problem here. But I sended you an exmaple, where <use> gave back the wrong strokeRect of the referenced object. We should talk about it in another bug report.
W. James MacLean
Comment 9 2010-06-24 07:23:03 PDT
I've tried a release build of Chrome-Linux on a modified version of Unix_history-simple.en.svg where I (1) change filterUnits to be "objectBoundingBox", (2) set x and y to be -0.3 and (3) set height & width to be 1.6. This renders very quickly, so this does seem to explain the performance issue.
Justin Schuh
Comment 10 2010-06-24 07:45:42 PDT
Dropping the security flag because this is confirmed non-exploitable.
David Kilzer (:ddkilzer)
Comment 11 2011-07-27 16:18:24 PDT
This works now with WebKit nightly build (r91865). Closing.
Note You need to log in before you can comment on or make changes to this bug.