[MERGE #4783 @boingoing] Fix a possible null dereference in PopulateM…

…etadataFromException Merge pull request #4783 from boingoing:FixPopulateMetadataFromExceptionNull If the code throwing the exception is dynamic (eval or otherwise doesn't have a url) the url of the `functionBody` can be null. In that case, we would pass null to JavascriptString::NewCopySz which dereferences it and crashes. Fix is to use GetSourceName instead which handles this case providing the string 'eval code' instead of nullptr.
chakra-core · Mar 6, 2018 · 1aee42c · 1aee42c
2 parents aef2cfa + 57b024e
commit 1aee42c
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 1 deletion.
diff --git a/bin/NativeTests/JsRTApiTest.cpp b/bin/NativeTests/JsRTApiTest.cpp
@@ -1091,6 +1091,48 @@ namespace JsRTApiTest
         REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
         REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
         CHECK(type == JsString);
+
+        // Following requires eval to be enabled - no point in testing it if we've disabled eval
+        if (!(attributes & JsRuntimeAttributeDisableEval))
+        {
+            REQUIRE(JsRunScript(_u("eval('var a = b');"), JS_SOURCE_CONTEXT_NONE, _u(""), nullptr) == JsErrorScriptException);
+            REQUIRE(JsHasException(&value) == JsNoError);
+            CHECK(value == true);
+
+            REQUIRE(JsGetAndClearExceptionWithMetadata(&exceptionMetadata) == JsNoError);
+            REQUIRE(JsHasException(&value) == JsNoError);
+            CHECK(value == false);
+
+            REQUIRE(JsGetPropertyIdFromName(_u("exception"), &property) == JsNoError);
+            REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
+            REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
+            CHECK(type == JsError);
+
+            REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
+            REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
+            REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
+            CHECK(type == JsNumber);
+
+            REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
+            REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
+            REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
+            CHECK(type == JsNumber);
+
+            REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
+            REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
+            REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
+            CHECK(type == JsNumber);
+
+            REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
+            REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
+            REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
+            CHECK(type == JsString);
+
+            REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
+            REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
+            REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
+            CHECK(type == JsString);
+        }
     }
 
     TEST_CASE("ApiTest_ExceptionHandlingTest", "[ApiTest]")

diff --git a/lib/Runtime/Library/JavascriptExceptionMetadata.cpp b/lib/Runtime/Library/JavascriptExceptionMetadata.cpp
@@ -91,7 +91,7 @@ namespace Js {
             Js::JavascriptNumber::New(0, scriptContext), scriptContext);
 
         Js::JavascriptOperators::OP_SetProperty(metadata, Js::PropertyIds::url,
-            Js::JavascriptString::NewCopySz(functionBody->GetSourceContextInfo()->url, scriptContext), scriptContext);
+            Js::JavascriptString::NewCopySz(functionBody->GetSourceName(), scriptContext), scriptContext);
 
         LPCUTF8 functionSource = sourceInfo->GetSource(_u("Jsrt::JsExperimentalGetAndClearExceptionWithMetadata"));