Bug 286307 – nullptr crash | WebCore::isScriptElement; WebCore::SVGAnimatedString::setBaseVal;

• Component: SVG
• Status: RESOLVED FIXED

[Adan Lopez]

We are calling isScriptElement with passing an element that is null so we needed to add a null check before calling that function.

Test case:
<p>This test passes if it doesn't crash.</p>
<script>
if (testRunner)
    testRunner.dumpAsText();

let animatedString = (() => {
    return (new Document).createElementNS('http://www.w3.org/2000/svg', 'text').className;
})();

if (window.GCController)
    GCController.collect();

animatedString.baseVal = 'foo';
let newValue = animatedString.baseVal;
document.write(newValue == 'foo' ? 'PASS' : `FAIL - ${newValue}`);

</script>


Backtrace:
#0 0x13ad0e43c in WebCore::isScriptElement(WebCore::Element&)+0x38 (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:arm64e+0x5d3643c)
    #1 0x13e7e023c in WebCore::SVGAnimatedString::setBaseVal(std::__1::variant<WTF::String, WTF::RefPtr<WebCore::TrustedScriptURL, WTF::RawPtrTraits<WebCore::TrustedScriptURL>, WTF::DefaultRefDerefTraits<WebCore::TrustedScriptURL>>> const&)+0xf4 (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:arm64e+0x980823c)
    #2 0x137464238 in WebCore::setJSSVGAnimatedString_baseValSetter(JSC::JSGlobalObject&, WebCore::JSSVGAnimatedString&, JSC::JSValue)+0x260 (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:arm64e+0x248c238)
    #3 0x1373c163c in WebCore::setJSSVGAnimatedString_baseVal(JSC::JSGlobalObject*, long long, long long, JSC::PropertyName)+0x14c (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:arm64e+0x23e963c)
    #4 0x11ffe8d2c in JSC::JSObject::putInlineSlow(JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)+0x6a4 (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/JavaScriptCore:arm64e+0x3250d2c)
    #5 0x11f715644 in operationPutByIdSloppyOptimize+0x6e0 (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/JavaScriptCore:arm64e+0x297d644)
    #6 0x1461b8110  (<unknown module>)
    #7 0x146181304  (<unknown module>)
    #8 0x14641f834  (<unknown module>)
    #9 0x14614c544  (<unknown module>)
    #10 0x11f4d5c94 in JSC::Interpreter::executeCall(JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x838 (/Users/lopezlozoya/repo/OpenSource/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/JavaScriptCore:arm64e+0x273dc94)

[Radar WebKit Bug Importer]

<rdar://problem/143330345>

[Darin Adler]

Is a null pointer dereference a heap-use-after-free?

[Darin Adler]

The fix to this null pointer dereference is super-simple:

isScriptElement(*el) needs to become isScriptElement(el) without the *.

I am surprised that it’s a security issue though. Seems more like a non-security fuzzing blocker to me.

[Adan Lopez]

Thanks for the feedback Darin, I'll try that approach. This was classified as Security from a fuzzer so I created the bug with the same classification.

[Darin Adler]

Sounds OK. I wonder how the fuzzer classifies things. It’s possible that it is right and I am wrong, but I would like to understand eventually.

[Adan Lopez]

Pull request: https://github.com/apple/WebKit/pull/2461

[Said Abou-Hallawa]

rdar://143330345

[Said Abou-Hallawa]

rdar://141021945

[Adan Lopez]

This was reclassified as non security bug. It is C/H/D bug.

[Adan Lopez]

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

[EWS]

Committed 289221@main (a722b348044c): <https://commits.webkit.org/289221@main>

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