429774 - Read past end of array [@ Convolve3x3] involving SVG feSpecularLighting filter

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase (often crashes Firefox when loaded)

Gary Kwong found this bug and I helped make a reduced testcase.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Robert Longson [:longsonr]]

Attachment: patch

This doesn't crash on Windows. 

There is a clear issue, however. y goes from rect.y to rect.YMost in the loop that calls GenerateNormal. We therefore need to change the kernel indexing when it gets to YMost rather than height. Only a problem if rect.y or rect.x <> 0, otherwise YMost == height which is why http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-light-01-f.html works.

Is it possible to confirm this patch stops the crash on Macs?

Comment 3

[Robert Longson [:longsonr]]

BTW thanks for the stack trace Jesse, that made it much easier.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Those == 0 tests should be testing rect.x and rect.y, right?

Comment 5

[Jesse Ruderman]

The patch fixes the crash for me.

Comment 6

[Robert Longson [:longsonr]]

Attachment: address review comment

(In reply to comment #4)
> Those == 0 tests should be testing rect.x and rect.y, right?
> 

Seems reasonable. We shouldn't read outside the source area. Although that fault won't make it crash.

Comment 7

[Robert Longson [:longsonr]]

Comment on attachment 316563 [details] [diff] [review]
address review comment

Simple fix to use correct bounds for reading.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 316563 [details] [diff] [review]
address review comment

a1.9=beltzner

Comment 9

[Robert Longson [:longsonr]]

checked in.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Verified that the testcase does not crash anymore in today's freshly compiled Mac debug builds.