Bug 297624 – REGRESSION(298250@main): feConvolveMatrix with large 'order' causes integer overflow

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Said Abou-Hallawa]

Created attachment 476459 [details]
test case

1. Open the attached test case in a debug build

Result: WebKit crashes with the following call stack:

#0	0x000000039bfdc609 in ::WTFCrash() at /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/Assertions.cpp:377
#1	0x00000003c70655c9 in WTF::CrashOnOverflow::crash at /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/CheckedArithmetic.h:110
#2	0x00000003c70655b9 in WTF::CrashOnOverflow::overflowed at /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/CheckedArithmetic.h:103
#3	0x00000003cc4b3c2e in WTF::Checked<unsigned int, WTF::CrashOnOverflow>::Checked at /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/CheckedArithmetic.h:643
#4	0x00000003cc4b3bcb in WTF::Checked<unsigned int, WTF::CrashOnOverflow>::Checked at /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/CheckedArithmetic.h:642
#5	0x00000003cc4b3b5c in WTF::operator*<unsigned int, int, WTF::CrashOnOverflow> at /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/CheckedArithmetic.h:879
#6	0x00000003cc4b3a94 in WTF::operator*<unsigned int, int, WTF::CrashOnOverflow> at /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/CheckedArithmetic.h:905
#7	0x00000003cc4b3a40 in WebCore::IntSize::area<WTF::CrashOnOverflow> at /Volumes/Data/WebKit/OpenSource/Source/WebCore/platform/graphics/IntSize.h:148
#8	0x00000003cec8b075 in WebCore::SVGFEConvolveMatrixElement::createFilterEffect at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/SVGFEConvolveMatrixElement.cpp:280
#9	0x00000003ced25d46 in WebCore::SVGFilterPrimitiveStandardAttributes::filterEffect at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/SVGFilterPrimitiveStandardAttributes.cpp:101
#10	0x00000003ceed03b9 in WebCore::buildFilterEffectGraph at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/graphics/filters/SVGFilter.cpp:103
#11	0x00000003ceecf994 in WebCore::SVGFilter::buildExpression at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/graphics/filters/SVGFilter.cpp:124
#12	0x00000003ceecf70c in WebCore::SVGFilter::create at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/graphics/filters/SVGFilter.cpp:43

Note: The attached file has an feConvolveMatrix filter. The 'order' attribute value of the feConvolveMatrix = "4, 1073741828". The product of these two signed integer overflows and gives a result = 16.

[Said Abou-Hallawa]

rdar://158670536

[Said Abou-Hallawa]

In fact the integer overflow is not a regression of 98250@main. The overflow could also have happened before 98250@main. The original code was this:

   // The spec says this is a requirement, and should bail out if fails
    if ((size_t)(orderXValue * orderYValue) != kernelMatrixSize)
        return nullptr;

The casting to size_t was wrong because the product (orderXValue * orderYValue) has to be signed integer and it can overflow. The new code is this:

    // The spec says this is a requirement, and should bail out if fails
    if (order.area() != kernelMatrix.length())
        return nullptr;

Calling IntSize::area() could have caught this possible overflow.

[Said Abou-Hallawa]

Pull request: https://github.com/WebKit/WebKit/pull/49621

[EWS]

Committed 298946@main (7f3400f5de0c): <https://commits.webkit.org/298946@main>

Reviewed commits have been landed. Closing PR #49621 and removing active labels.