[JSC] NumberPrototype::extractRadixFromArgs incorrectly cast double to int32_t

https://bugs.webkit.org/show_bug.cgi?id=181182

Reviewed by Darin Adler.

JSTests:

* stress/big-int-prototype-to-string-cast-overflow.js: Added.
* stress/big-int-prototype-to-string-exception.js: Added.
* stress/big-int-prototype-to-string-wrong-values.js: Added.
* stress/number-prototype-to-string-cast-overflow.js: Added.
* stress/number-prototype-to-string-exception.js: Added.
* stress/number-prototype-to-string-wrong-values.js: Added.

Source/JavaScriptCore:

Casting double to integer is undefined behavior when the truncation
results into a value that doesn't fit into integer size,
according C++ spec[1]. Thus, we are changing bigIntProtoFuncToString and
numberProtoFuncToString to remove these source of undefined
behavior.

[1] - http://en.cppreference.com/w/cpp/language/implicit_conversion

* runtime/BigIntPrototype.cpp:
(JSC::bigIntProtoFuncToString):
* runtime/NumberPrototype.cpp:
(JSC::numberProtoFuncToString):
(JSC::extractToStringRadixArgument):
(JSC::extractRadixFromArgs): Deleted.
* runtime/NumberPrototype.h:

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

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 0e05907..c12cab4 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,17 @@
+2018-01-20  Caio Lima  <ticaiolima@gmail.com>
+
+        [JSC] NumberPrototype::extractRadixFromArgs incorrectly cast double to int32_t
+        https://bugs.webkit.org/show_bug.cgi?id=181182
+
+        Reviewed by Darin Adler.
+
+        * stress/big-int-prototype-to-string-cast-overflow.js: Added.
+        * stress/big-int-prototype-to-string-exception.js: Added.
+        * stress/big-int-prototype-to-string-wrong-values.js: Added.
+        * stress/number-prototype-to-string-cast-overflow.js: Added.
+        * stress/number-prototype-to-string-exception.js: Added.
+        * stress/number-prototype-to-string-wrong-values.js: Added.
+
 2018-01-19  Ryan Haddad  <ryanhaddad@apple.com>
 
         Disable Atomics when SharedArrayBuffer isn’t enabled

diff --git a/JSTests/stress/big-int-prototype-to-string-cast-overflow.js b/JSTests/stress/big-int-prototype-to-string-cast-overflow.js
new file mode 100644 (file)
index 0000000..a9fa302
--- /dev/null
+++ b/JSTests/stress/big-int-prototype-to-string-cast-overflow.js
@@ -0,0 +1,20 @@
+//@ runBigIntEnabled
+
+function assert(a) {
+    if (!a)
+        throw new Error("Bad assertion");
+}
+
+function assertThrowRangeError(input) {
+    try {
+        let number = 3n;
+        number.toString(input);
+        assert(false);
+    } catch (e) {
+        assert(e instanceof RangeError);
+    }
+}
+
+assertThrowRangeError(1e100);
+assertThrowRangeError(-1e101);
+

diff --git a/JSTests/stress/big-int-prototype-to-string-exception.js b/JSTests/stress/big-int-prototype-to-string-exception.js
new file mode 100644 (file)
index 0000000..6c1d3c0
--- /dev/null
+++ b/JSTests/stress/big-int-prototype-to-string-exception.js
@@ -0,0 +1,22 @@
+//@ runBigIntEnabled
+
+function assert(a) {
+    if (!a)
+        throw new Error("Bad assertion");
+}
+
+let o = {
+    valueOf: () => {
+        throw new Error("Bad");
+        return 2;
+    }
+}
+
+let a = 20n;
+try {
+    a.toString(o);
+    assert(false);
+} catch (e) {
+    assert(e.message == "Bad");
+}
+

diff --git a/JSTests/stress/big-int-prototype-to-string-wrong-values.js b/JSTests/stress/big-int-prototype-to-string-wrong-values.js
new file mode 100644 (file)
index 0000000..79e51d5
--- /dev/null
+++ b/JSTests/stress/big-int-prototype-to-string-wrong-values.js
@@ -0,0 +1,31 @@
+//@ runBigIntEnabled
+
+function assert(a) {
+    if (!a)
+        throw new Error("Bad assertion");
+}
+
+function assertRangeError(v) {
+    let a = 456n;
+    try {
+        a.toString(v);
+        assert(false);
+    } catch (e) {
+        assert(e instanceof RangeError);
+    }
+}
+
+assertRangeError(1);
+assertRangeError(37);
+assertRangeError(37.1);
+assertRangeError(37.2);
+assertRangeError(0);
+assertRangeError(-1);
+assertRangeError(1.999999);
+assertRangeError(37.00000000000000001);
+assertRangeError(NaN);
+assertRangeError(null);
+assertRangeError(+Infinity);
+assertRangeError(-Infinity);
+assertRangeError(-0);
+

diff --git a/JSTests/stress/number-prototype-to-string-cast-overflow.js b/JSTests/stress/number-prototype-to-string-cast-overflow.js
new file mode 100644 (file)
index 0000000..f12ab05
--- /dev/null
+++ b/JSTests/stress/number-prototype-to-string-cast-overflow.js
@@ -0,0 +1,18 @@
+function assert(a) {
+    if (!a)
+        throw new Error("Bad assertion");
+}
+
+function assertThrowRangeError(input) {
+    try {
+        let number = 3;
+        number.toString(input);
+        assert(false);
+    } catch (e) {
+        assert(e instanceof RangeError);
+    }
+}
+
+assertThrowRangeError(1e100);
+assertThrowRangeError(-1e101);
+

diff --git a/JSTests/stress/number-prototype-to-string-exception.js b/JSTests/stress/number-prototype-to-string-exception.js
new file mode 100644 (file)
index 0000000..7445907
--- /dev/null
+++ b/JSTests/stress/number-prototype-to-string-exception.js
@@ -0,0 +1,20 @@
+function assert(a) {
+    if (!a)
+        throw new Error("Bad assertion");
+}
+
+let o = {
+    valueOf: () => {
+        throw new Error("Bad");
+        return 2;
+    }
+}
+
+let a = 2;
+try {
+    a.toString(o);
+    assert(false);
+} catch (e) {
+    assert(e.message == "Bad");
+}
+

diff --git a/JSTests/stress/number-prototype-to-string-wrong-values.js b/JSTests/stress/number-prototype-to-string-wrong-values.js
new file mode 100644 (file)
index 0000000..52e6e11
--- /dev/null
+++ b/JSTests/stress/number-prototype-to-string-wrong-values.js
@@ -0,0 +1,29 @@
+function assert(a) {
+    if (!a)
+        throw new Error("Bad assertion");
+}
+
+function assertRangeError(v) {
+    let a = 2;
+    try {
+        a.toString(v);
+        assert(false);
+    } catch (e) {
+        assert(e instanceof RangeError);
+    }
+}
+
+assertRangeError(1);
+assertRangeError(37);
+assertRangeError(37.1);
+assertRangeError(37.2);
+assertRangeError(0);
+assertRangeError(-1);
+assertRangeError(1.999999);
+assertRangeError(37.00000000000000001);
+assertRangeError(NaN);
+assertRangeError(null);
+assertRangeError(+Infinity);
+assertRangeError(-Infinity);
+assertRangeError(-0);
+

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 70fab60..f18b8f1 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+2018-01-20  Caio Lima  <ticaiolima@gmail.com>
+
+        [JSC] NumberPrototype::extractRadixFromArgs incorrectly cast double to int32_t
+        https://bugs.webkit.org/show_bug.cgi?id=181182
+
+        Reviewed by Darin Adler.
+
+        Casting double to integer is undefined behavior when the truncation
+        results into a value that doesn't fit into integer size,
+        according C++ spec[1]. Thus, we are changing bigIntProtoFuncToString and
+        numberProtoFuncToString to remove these source of undefined
+        behavior.
+
+        [1] - http://en.cppreference.com/w/cpp/language/implicit_conversion
+
+        * runtime/BigIntPrototype.cpp:
+        (JSC::bigIntProtoFuncToString):
+        * runtime/NumberPrototype.cpp:
+        (JSC::numberProtoFuncToString):
+        (JSC::extractToStringRadixArgument):
+        (JSC::extractRadixFromArgs): Deleted.
+        * runtime/NumberPrototype.h:
+
 2018-01-19  Saam Barati  <sbarati@apple.com>
 
         Kill ArithNegate's ArithProfile assert inside BytecodeParser

diff --git a/Source/JavaScriptCore/runtime/BigIntPrototype.cpp b/Source/JavaScriptCore/runtime/BigIntPrototype.cpp
index bb903e4..6435cc8 100644 (file)
--- a/Source/JavaScriptCore/runtime/BigIntPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/BigIntPrototype.cpp
@@ -35,6 +35,7 @@
 #include "JSFunction.h"
 #include "JSGlobalObject.h"
 #include "JSString.h"
+#include "NumberPrototype.h"
 #include <wtf/Assertions.h>
 
 namespace JSC {
@@ -99,26 +100,15 @@ EncodedJSValue JSC_HOST_CALL bigIntProtoFuncToString(ExecState* state)
     
     ASSERT(value);
 
-    int64_t radix;
-    JSValue radixValue = state->argument(0);
-    if (radixValue.isInt32())
-        radix = radixValue.asInt32();
-    else if (radixValue.isUndefined())
-        radix = 10;
-    else {
-        radix = static_cast<int64_t>(radixValue.toInteger(state));
-        RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    }
-
-    if (radix < 2 || radix > 36)
-        return throwVMError(state, scope, createRangeError(state, ASCIILiteral("toString() radix argument must be between 2 and 36")));
+    int32_t radix = extractToStringRadixArgument(state, state->argument(0), scope);
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
-    String resultString = value->toString(*state, static_cast<int32_t>(radix));
+    String resultString = value->toString(*state, radix);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    scope.release();
     if (resultString.length() == 1)
         return JSValue::encode(vm.smallStrings.singleCharacterString(resultString[0]));
 
-    scope.release();
     return JSValue::encode(jsNontrivialString(&vm, resultString));
 }
 

diff --git a/Source/JavaScriptCore/runtime/NumberPrototype.cpp b/Source/JavaScriptCore/runtime/NumberPrototype.cpp
index a63d01e..40de036 100644 (file)
--- a/Source/JavaScriptCore/runtime/NumberPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/NumberPrototype.cpp
@@ -504,20 +504,6 @@ EncodedJSValue JSC_HOST_CALL numberProtoFuncToPrecision(ExecState* exec)
     return JSValue::encode(jsString(exec, String(numberToFixedPrecisionString(x, significantFigures, buffer))));
 }
 
-static inline int32_t extractRadixFromArgs(ExecState* exec)
-{
-    JSValue radixValue = exec->argument(0);
-    int32_t radix;
-    if (radixValue.isInt32())
-        radix = radixValue.asInt32();
-    else if (radixValue.isUndefined())
-        radix = 10;
-    else
-        radix = static_cast<int32_t>(radixValue.toInteger(exec)); // nan -> 0
-
-    return radix;
-}
-
 static ALWAYS_INLINE JSString* int32ToStringInternal(VM& vm, int32_t value, int32_t radix)
 {
     ASSERT(!(radix < 2 || radix > 36));
@@ -586,21 +572,18 @@ JSString* numberToString(VM& vm, double doubleValue, int32_t radix)
     return numberToStringInternal(vm, doubleValue, radix);
 }
 
-EncodedJSValue JSC_HOST_CALL numberProtoFuncToString(ExecState* exec)
+EncodedJSValue JSC_HOST_CALL numberProtoFuncToString(ExecState* state)
 {
-    VM& vm = exec->vm();
+    VM& vm = state->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     double doubleValue;
-    if (!toThisNumber(exec->thisValue(), doubleValue))
-        return throwVMTypeError(exec, scope);
+    if (!toThisNumber(state->thisValue(), doubleValue))
+        return throwVMTypeError(state, scope);
 
-    int32_t radix = extractRadixFromArgs(exec);
+    auto radix = extractToStringRadixArgument(state, state->argument(0), scope);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    if (radix < 2 || radix > 36)
-        return throwVMError(exec, scope, createRangeError(exec, ASCIILiteral("toString() radix argument must be between 2 and 36")));
 
-    scope.release();
     return JSValue::encode(numberToStringInternal(vm, doubleValue, radix));
 }
 
@@ -628,4 +611,24 @@ EncodedJSValue JSC_HOST_CALL numberProtoFuncValueOf(ExecState* exec)
     return JSValue::encode(jsNumber(x));
 }
 
+int32_t extractToStringRadixArgument(ExecState* state, JSValue radixValue, ThrowScope& throwScope)
+{
+    if (radixValue.isUndefined())
+        return 10;
+
+    if (radixValue.isInt32()) {
+        int32_t radix = radixValue.asInt32();
+        if (radix >= 2 && radix <= 36)
+            return radix;
+    } else {
+        double radixDouble = radixValue.toInteger(state);
+        RETURN_IF_EXCEPTION(throwScope, 0);
+        if (radixDouble >= 2 && radixDouble <= 36)
+            return static_cast<int32_t>(radixDouble);   
+    }
+
+    throwRangeError(state, throwScope, ASCIILiteral("toString() radix argument must be between 2 and 36"));
+    return 0;
+}
+
 } // namespace JSC

diff --git a/Source/JavaScriptCore/runtime/NumberPrototype.h b/Source/JavaScriptCore/runtime/NumberPrototype.h
index 039b0d5..b353c75 100644 (file)
--- a/Source/JavaScriptCore/runtime/NumberPrototype.h
+++ b/Source/JavaScriptCore/runtime/NumberPrototype.h
@@ -55,5 +55,6 @@ JSString* int32ToString(VM&, int32_t value, int32_t radix);
 JSString* int52ToString(VM&, int64_t value, int32_t radix);
 JSString* numberToString(VM&, double value, int32_t radix);
 String toStringWithRadix(double doubleValue, int32_t radix);
+int32_t extractToStringRadixArgument(ExecState*, JSValue radixValue, ThrowScope&);
 
 } // namespace JSC