String overflow when using StringBuilder in JSC::createError
authordinfuehr@igalia.com <dinfuehr@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Mar 2019 21:42:17 +0000 (21:42 +0000)
committerdinfuehr@igalia.com <dinfuehr@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Mar 2019 21:42:17 +0000 (21:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194957

Reviewed by Mark Lam.

JSTests:

Add test string-overflow-createError-bulder.js that overflows
StringBuilder in notAFunctionSourceAppender. The second new test
string-overflow-createError-fit.js has an error message that doesn't
overflow, it still failed since the String's capacity can't be doubled.
Run test string-overflow-createError.js only in the default
configuration to reduce memory consumption when running the test
in all configurations on multiple CPUs in parallel.

* stress/string-overflow-createError-builder.js: Copied from JSTests/stress/string-overflow-createError.js.
(catch):
* stress/string-overflow-createError-fit.js: Copied from JSTests/stress/string-overflow-createError.js.
(catch):
* stress/string-overflow-createError.js:

Source/JavaScriptCore:

StringBuilder in notAFunctionSourceAppender didn't check
for overflows but just failed.

* runtime/ExceptionHelpers.cpp:
(JSC::notAFunctionSourceAppender):

Source/WTF:

When calculating the new capacity of a StringBuilder object,
use a limit of MaxLength instead of MaxLength+1.  Allocating
a string of size MaxLength+1 always fails. This means that expanding
a StringBuilder only worked when the newly doubled capacity is less or
equal to MaxLength.

* wtf/text/StringBuilder.cpp:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242910 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/string-overflow-createError-builder.js [new file with mode: 0644]
JSTests/stress/string-overflow-createError-fit.js [new file with mode: 0644]
JSTests/stress/string-overflow-createError.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringBuilder.cpp

index a6300d9..cedacfd 100644 (file)
@@ -1,3 +1,24 @@
+2019-03-13  Dominik Infuehr  <dinfuehr@igalia.com>
+
+        String overflow when using StringBuilder in JSC::createError
+        https://bugs.webkit.org/show_bug.cgi?id=194957
+
+        Reviewed by Mark Lam.
+
+        Add test string-overflow-createError-bulder.js that overflows
+        StringBuilder in notAFunctionSourceAppender. The second new test
+        string-overflow-createError-fit.js has an error message that doesn't
+        overflow, it still failed since the String's capacity can't be doubled.
+        Run test string-overflow-createError.js only in the default
+        configuration to reduce memory consumption when running the test
+        in all configurations on multiple CPUs in parallel.
+
+        * stress/string-overflow-createError-builder.js: Copied from JSTests/stress/string-overflow-createError.js.
+        (catch):
+        * stress/string-overflow-createError-fit.js: Copied from JSTests/stress/string-overflow-createError.js.
+        (catch):
+        * stress/string-overflow-createError.js:
+
 2019-03-12  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] OSR entry should respect abstract values in addition to flush formats
diff --git a/JSTests/stress/string-overflow-createError-builder.js b/JSTests/stress/string-overflow-createError-builder.js
new file mode 100644 (file)
index 0000000..7af309e
--- /dev/null
@@ -0,0 +1,15 @@
+//@ skip if $memoryLimited
+//@ runDefault
+var exception;
+try {
+    bar = '2.3023e-320'
+    foo = bar.padEnd(2147483620, 1);
+    foo(true, 1).value;
+} catch (e) {
+    exception = e;
+}
+
+// when the StringBuilder for the error message overflows,
+// "object is not a function" is used as message for the TypeError.
+if (exception.message != "object is not a function.")
+    throw "FAILED";
diff --git a/JSTests/stress/string-overflow-createError-fit.js b/JSTests/stress/string-overflow-createError-fit.js
new file mode 100644 (file)
index 0000000..dc05937
--- /dev/null
@@ -0,0 +1,16 @@
+//@ skip if $memoryLimited
+//@ runDefault
+var exception;
+try {
+    bar = '2.3023e-320'
+    foo = bar.padEnd(2147480000, 1);
+    foo(true, 1).value;
+} catch (e) {
+    exception = e;
+}
+
+// Although the message of the TypeError is quite long,
+// it still fits into String::MaxLength. Check the start
+// of the error message.
+if (!exception.message.startsWith("foo is not a function"))
+    throw "FAILED";
index 97ab1fa..80b93b5 100644 (file)
@@ -1,4 +1,5 @@
 //@ skip if $memoryLimited
+//@ runDefault
 var exception;
 try {
     bar = '2.3023e-320'
@@ -8,5 +9,7 @@ try {
     exception = e;
 }
 
+// Creating the error message for the TypeError overflows
+// the string and therefore an out-of-memory error is thrown.
 if (exception != "Error: Out of memory")
     throw "FAILED";
index ebef9a3..c7909cf 100644 (file)
@@ -1,3 +1,16 @@
+2019-03-13  Dominik Infuehr  <dinfuehr@igalia.com>
+
+        String overflow when using StringBuilder in JSC::createError
+        https://bugs.webkit.org/show_bug.cgi?id=194957
+
+        Reviewed by Mark Lam.
+
+        StringBuilder in notAFunctionSourceAppender didn't check
+        for overflows but just failed.
+
+        * runtime/ExceptionHelpers.cpp:
+        (JSC::notAFunctionSourceAppender):
+
 2019-03-11  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Move species watchpoint installation from ArrayPrototype to JSGlobalObject
index 4ea4497..95ae44c 100644 (file)
@@ -193,7 +193,7 @@ static String notAFunctionSourceAppender(const String& originalMessage, const St
     String base = functionCallBase(sourceText);
     if (!base)
         return defaultApproximateSourceError(originalMessage, sourceText);
-    StringBuilder builder;
+    StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
     builder.append(base);
     builder.appendLiteral(" is not a function. (In '");
     builder.append(sourceText);
@@ -209,6 +209,9 @@ static String notAFunctionSourceAppender(const String& originalMessage, const St
     }
     builder.append(')');
 
+    if (builder.hasOverflowed())
+        return makeString("object is not a function."_s);
+
     return builder.toString();
 }
 
index 57f2df5..ea36d40 100644 (file)
@@ -1,3 +1,18 @@
+2019-03-13  Dominik Infuehr  <dinfuehr@igalia.com>
+
+        String overflow when using StringBuilder in JSC::createError
+        https://bugs.webkit.org/show_bug.cgi?id=194957
+
+        Reviewed by Mark Lam.
+
+        When calculating the new capacity of a StringBuilder object,
+        use a limit of MaxLength instead of MaxLength+1.  Allocating
+        a string of size MaxLength+1 always fails. This means that expanding
+        a StringBuilder only worked when the newly doubled capacity is less or
+        equal to MaxLength.
+
+        * wtf/text/StringBuilder.cpp:
+
 2019-03-13  Chris Dumez  <cdumez@apple.com>
 
         Better build fix after r242901.
index be1ccc7..1489adf 100644 (file)
@@ -34,7 +34,7 @@
 
 namespace WTF {
 
-static constexpr unsigned maxCapacity = String::MaxLength + 1;
+static constexpr unsigned maxCapacity = String::MaxLength;
 
 static unsigned expandedCapacity(unsigned capacity, unsigned requiredLength)
 {