Correctly detect string overflow when using the 'Function' constructor.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Oct 2018 02:58:51 +0000 (02:58 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Oct 2018 02:58:51 +0000 (02:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184883
<rdar://problem/36320331>

Reviewed by Saam Barati.

JSTests:

I've verified that this passes on 32-bit as well.

* slowMicrobenchmarks/function-constructor-with-huge-strings.js: Added.

Source/bmalloc:

* bmalloc/Allocator.cpp:
(bmalloc::Allocator::reallocate):
(bmalloc::Allocator::tryReallocate):
(bmalloc::Allocator::reallocateImpl):
* bmalloc/Allocator.h:
* bmalloc/Cache.h:
(bmalloc::Cache::tryReallocate):
* bmalloc/DebugHeap.cpp:
(bmalloc::DebugHeap::realloc):
* bmalloc/DebugHeap.h:
* bmalloc/bmalloc.h:
(bmalloc::api::tryRealloc):

Source/JavaScriptCore:

Added StringBuilder::hasOverflowed() checks, and throwing OutOfMemoryErrors if
we detect an overflow.

* runtime/FunctionConstructor.cpp:
(JSC::constructFunctionSkippingEvalEnabledCheck):
* runtime/JSGlobalObjectFunctions.cpp:
(JSC::encode):
(JSC::decode):
* runtime/JSONObject.cpp:
(JSC::Stringifier::stringify):
(JSC::Stringifier::appendStringifiedValue):

Source/WTF:

1. Enhanced StringBuilder so that it supports 2 modes of behavior:
   a. StringBuilder::OverflowHandler::CrashOnOverflow.
   b. StringBuilder::OverflowHandler::RecordOverflow.

   CrashOnOverflow will crash the moment an overflow or failure to allocate
   memory is detected.

   RecordOverflow will make StringBuilder::hasOverflowed() return true when an
   overflow or failure to allocate memory is detected.  However, if the user
   invokes toString(), toStringPreserveCapacity(), toAtomicString(), length(),
   capacity(), isEmpty(), characters8(), or characters16(), then the StringBuilder
   will crash regardless because these methods can only meaningfully do their
   work and return a result if and only if the builder has not overflowed.

   By default, the StringBuilder crashes on overflow (the old behavior) unless it
   is explicitly constructed with the StringBuilder::OverflowHandler::RecordOverflow
   enum.

   Design note:

   The StringBuilder can only be put in 1 of the 2 modes at the time of
   construction.  This means that we could have implemented it as a template
   that is parameterized on an OverflowHandler class (like CheckedArithmetic)
   instead of using a runtime check in the ConditionalCrashOnOverflow handler.

   However, I was not able to get clang to export the explicitly instantiated
   instances (despite the methods having being decorated with WTF_EXPORT_PRIVATE).
   Since the StringBuilder is a transient object (not stored for a long time on
   the heap), and the runtime check of the boolean is not that costly compared
   to other work that StringBuilder routinely does (e.g. memcpy), using the
   new ConditionalCrashOnOverflow (which does a runtime check to determine to
   crash or not on overflow) is fine.

   When clang is able to export explicitly instantiated template methods, we can
   templatize StringBuilder and do away with ConditionalCrashOnOverflow.
   See https://bugs.webkit.org/show_bug.cgi?id=191050.

To support the above, we also did:

2. Enhanced all StringBuilder append and buffer allocation methods to be able to
   fail without crashing.  This also meant that ...

3. Added tryReallocate() support to StringImpl.  tryRealloc() was added to
   FastMalloc, and bmalloc (and related peers) to enable this.

4. Added ConditionalCrashOnOverflow, which can choose at runtime whether to behave
   like CrashOnOverflow or RecordOverflow.

* wtf/CheckedArithmetic.h:
(WTF::ConditionalCrashOnOverflow::overflowed):
(WTF::ConditionalCrashOnOverflow::shouldCrashOnOverflow const):
(WTF::ConditionalCrashOnOverflow::setShouldCrashOnOverflow):
(WTF::ConditionalCrashOnOverflow::hasOverflowed const):
(WTF::ConditionalCrashOnOverflow::clearOverflow):
(WTF::ConditionalCrashOnOverflow::crash):
(WTF::RecordOverflow::overflowed):
(WTF::Checked::unsafeGet const):
* wtf/FastMalloc.cpp:
(WTF::tryFastRealloc):
* wtf/FastMalloc.h:
* wtf/text/StringBuilder.cpp:
(WTF::expandedCapacity):
(WTF::StringBuilder::reifyString const):
(WTF::StringBuilder::resize):
(WTF::StringBuilder::allocateBuffer):
(WTF::StringBuilder::allocateBufferUpConvert):
(WTF::StringBuilder::reallocateBuffer<LChar>):
(WTF::StringBuilder::reallocateBuffer<UChar>):
(WTF::StringBuilder::reserveCapacity):
(WTF::StringBuilder::appendUninitialized):
(WTF::StringBuilder::appendUninitializedSlow):
(WTF::StringBuilder::append):
(WTF::StringBuilder::canShrink const):
(WTF::StringBuilder::shrinkToFit):
* wtf/text/StringBuilder.h:
(WTF::StringBuilder::StringBuilder):
(WTF::StringBuilder::didOverflow):
(WTF::StringBuilder::hasOverflowed const):
(WTF::StringBuilder::crashesOnOverflow const):
(WTF::StringBuilder::append):
(WTF::StringBuilder::toString):
(WTF::StringBuilder::toStringPreserveCapacity const):
(WTF::StringBuilder::toAtomicString const):
(WTF::StringBuilder::length const):
(WTF::StringBuilder::capacity const):
(WTF::StringBuilder::operator[] const):
(WTF::StringBuilder::swap):
(WTF::StringBuilder::getBufferCharacters<UChar>):
* wtf/text/StringBuilderJSON.cpp:
(WTF::StringBuilder::appendQuotedJSONString):
* wtf/text/StringImpl.cpp:
(WTF::StringImpl::reallocateInternal):
(WTF::StringImpl::reallocate):
(WTF::StringImpl::tryReallocate):
* wtf/text/StringImpl.h:

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

22 files changed:
JSTests/ChangeLog
JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/FunctionConstructor.cpp
Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
Source/JavaScriptCore/runtime/JSONObject.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/CheckedArithmetic.h
Source/WTF/wtf/FastMalloc.cpp
Source/WTF/wtf/FastMalloc.h
Source/WTF/wtf/text/StringBuilder.cpp
Source/WTF/wtf/text/StringBuilder.h
Source/WTF/wtf/text/StringBuilderJSON.cpp
Source/WTF/wtf/text/StringImpl.cpp
Source/WTF/wtf/text/StringImpl.h
Source/bmalloc/ChangeLog
Source/bmalloc/bmalloc/Allocator.cpp
Source/bmalloc/bmalloc/Allocator.h
Source/bmalloc/bmalloc/Cache.h
Source/bmalloc/bmalloc/DebugHeap.cpp
Source/bmalloc/bmalloc/DebugHeap.h
Source/bmalloc/bmalloc/bmalloc.h

index d956979..ef2219c 100644 (file)
@@ -1,3 +1,15 @@
+2018-10-29  Mark Lam  <mark.lam@apple.com>
+
+        Correctly detect string overflow when using the 'Function' constructor.
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        <rdar://problem/36320331>
+
+        Reviewed by Saam Barati.
+
+        I've verified that this passes on 32-bit as well.
+
+        * slowMicrobenchmarks/function-constructor-with-huge-strings.js: Added.
+
 2018-10-29  Tadeu Zagallo  <tzagallo@apple.com>
 
         Add support for GetStack FlushedDouble
diff --git a/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js b/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js
new file mode 100644 (file)
index 0000000..c613ed8
--- /dev/null
@@ -0,0 +1,19 @@
+var hugeString = "x";
+for (i = 0; i < 25; ++i) {
+    hugeString += hugeString;
+}
+
+var exception;
+var weird = 'Â\8f';
+try {
+    var f = new Function(hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
+      hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
+      hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
+      () => 42,
+      "return 42;");
+} catch (e) {
+    exception = e;
+}
+
+if (exception != "Error: Out of memory")
+    throw "FAIL";
index 1c0914e..bacf61c 100644 (file)
@@ -1,3 +1,23 @@
+2018-10-29  Mark Lam  <mark.lam@apple.com>
+
+        Correctly detect string overflow when using the 'Function' constructor.
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        <rdar://problem/36320331>
+
+        Reviewed by Saam Barati.
+
+        Added StringBuilder::hasOverflowed() checks, and throwing OutOfMemoryErrors if
+        we detect an overflow.
+
+        * runtime/FunctionConstructor.cpp:
+        (JSC::constructFunctionSkippingEvalEnabledCheck):
+        * runtime/JSGlobalObjectFunctions.cpp:
+        (JSC::encode):
+        (JSC::decode):
+        * runtime/JSONObject.cpp:
+        (JSC::Stringifier::stringify):
+        (JSC::Stringifier::appendStringifiedValue):
+
 2018-10-29  Tadeu Zagallo  <tzagallo@apple.com>
 
         Unreviewed, fix JSC on arm64e after r237547
index 265049f..3be41a1 100644 (file)
@@ -123,21 +123,25 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(
         RETURN_IF_EXCEPTION(scope, nullptr);
         program = makeString("{", prefix, functionName.string(), "() {\n", body, "\n}}");
     } else {
-        StringBuilder builder;
+        StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
         builder.append('{');
         builder.append(prefix);
         builder.append(functionName.string());
         builder.append('(');
-        StringBuilder parameterBuilder;
+        StringBuilder parameterBuilder(StringBuilder::OverflowHandler::RecordOverflow);
         auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
         parameterBuilder.append(viewWithString.view);
-        for (size_t i = 1; i < args.size() - 1; i++) {
+        for (size_t i = 1; !parameterBuilder.hasOverflowed() && i < args.size() - 1; i++) {
             parameterBuilder.appendLiteral(", ");
             auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(exec);
             RETURN_IF_EXCEPTION(scope, nullptr);
             parameterBuilder.append(viewWithString.view);
         }
+        if (parameterBuilder.hasOverflowed()) {
+            throwOutOfMemoryError(exec, scope);
+            return nullptr;
+        }
         auto body = args.at(args.size() - 1).toWTFString(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
 
@@ -164,6 +168,10 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(
         RETURN_IF_EXCEPTION(scope, nullptr);
         builder.append(body);
         builder.appendLiteral("\n}}");
+        if (builder.hasOverflowed()) {
+            throwOutOfMemoryError(exec, scope);
+            return nullptr;
+        }
         program = builder.toString();
     }
 
index eaf130e..2b9f92d 100644 (file)
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999-2002 Harri Porten (porten@kde.org)
  *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
- *  Copyright (C) 2003-2017 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003-2018 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Cameron Zwarich (cwzwarich@uwaterloo.ca)
  *  Copyright (C) 2007 Maks Orlovich
  *
@@ -87,7 +87,7 @@ static JSValue encode(ExecState* exec, const Bitmap<256>& doNotEscape, const Cha
         return JSC::throwException(exec, scope, createURIError(exec, "String contained an illegal UTF-16 sequence."_s));
     };
 
-    StringBuilder builder;
+    StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
     builder.reserveCapacity(length);
 
     // 4. Repeat
@@ -151,6 +151,8 @@ static JSValue encode(ExecState* exec, const Bitmap<256>& doNotEscape, const Cha
         }
     }
 
+    if (UNLIKELY(builder.hasOverflowed()))
+        return throwOutOfMemoryError(exec, scope);
     return jsString(exec, builder.toString());
 }
 
@@ -170,7 +172,7 @@ static JSValue decode(ExecState* exec, const CharType* characters, int length, c
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    StringBuilder builder;
+    StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
     int k = 0;
     UChar u = 0;
     while (k < length) {
@@ -229,6 +231,8 @@ static JSValue decode(ExecState* exec, const CharType* characters, int length, c
         k++;
         builder.append(c);
     }
+    if (UNLIKELY(builder.hasOverflowed()))
+        return throwOutOfMemoryError(exec, scope);
     RELEASE_AND_RETURN(scope, jsString(&vm, builder.toString()));
 }
 
index 3dc2b8f..779fe64 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -273,7 +273,7 @@ JSValue Stringifier::stringify(JSValue value)
         object->putDirect(vm, vm.propertyNames->emptyIdentifier, value);
     }
 
-    StringBuilder result;
+    StringBuilder result(StringBuilder::OverflowHandler::RecordOverflow);
     Holder root(Holder::RootHolder, object);
     auto stringifyResult = appendStringifiedValue(result, value, root, emptyPropertyName);
     EXCEPTION_ASSERT(!scope.exception() || (stringifyResult != StringifySucceeded));
@@ -358,10 +358,12 @@ Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder&
     if (value.isString()) {
         const String& string = asString(value)->value(m_exec);
         RETURN_IF_EXCEPTION(scope, StringifyFailed);
-        if (builder.appendQuotedJSONString(string))
-            return StringifySucceeded;
-        throwOutOfMemoryError(m_exec, scope);
-        return StringifyFailed;
+        builder.appendQuotedJSONString(string);
+        if (UNLIKELY(builder.hasOverflowed())) {
+            throwOutOfMemoryError(m_exec, scope);
+            return StringifyFailed;
+        }
+        return StringifySucceeded;
     }
 
     if (value.isNumber()) {
index 5faedb0..4fd2e1d 100644 (file)
@@ -1,3 +1,107 @@
+2018-10-29  Mark Lam  <mark.lam@apple.com>
+
+        Correctly detect string overflow when using the 'Function' constructor.
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        <rdar://problem/36320331>
+
+        Reviewed by Saam Barati.
+
+        1. Enhanced StringBuilder so that it supports 2 modes of behavior:
+           a. StringBuilder::OverflowHandler::CrashOnOverflow.
+           b. StringBuilder::OverflowHandler::RecordOverflow.
+
+           CrashOnOverflow will crash the moment an overflow or failure to allocate
+           memory is detected.
+
+           RecordOverflow will make StringBuilder::hasOverflowed() return true when an
+           overflow or failure to allocate memory is detected.  However, if the user
+           invokes toString(), toStringPreserveCapacity(), toAtomicString(), length(),
+           capacity(), isEmpty(), characters8(), or characters16(), then the StringBuilder
+           will crash regardless because these methods can only meaningfully do their
+           work and return a result if and only if the builder has not overflowed.
+
+           By default, the StringBuilder crashes on overflow (the old behavior) unless it
+           is explicitly constructed with the StringBuilder::OverflowHandler::RecordOverflow
+           enum.
+
+           Design note:
+
+           The StringBuilder can only be put in 1 of the 2 modes at the time of
+           construction.  This means that we could have implemented it as a template
+           that is parameterized on an OverflowHandler class (like CheckedArithmetic)
+           instead of using a runtime check in the ConditionalCrashOnOverflow handler.
+
+           However, I was not able to get clang to export the explicitly instantiated
+           instances (despite the methods having being decorated with WTF_EXPORT_PRIVATE).
+           Since the StringBuilder is a transient object (not stored for a long time on
+           the heap), and the runtime check of the boolean is not that costly compared
+           to other work that StringBuilder routinely does (e.g. memcpy), using the
+           new ConditionalCrashOnOverflow (which does a runtime check to determine to
+           crash or not on overflow) is fine.
+
+           When clang is able to export explicitly instantiated template methods, we can
+           templatize StringBuilder and do away with ConditionalCrashOnOverflow.
+           See https://bugs.webkit.org/show_bug.cgi?id=191050.
+
+        To support the above, we also did:
+
+        2. Enhanced all StringBuilder append and buffer allocation methods to be able to
+           fail without crashing.  This also meant that ...
+
+        3. Added tryReallocate() support to StringImpl.  tryRealloc() was added to
+           FastMalloc, and bmalloc (and related peers) to enable this.
+
+        4. Added ConditionalCrashOnOverflow, which can choose at runtime whether to behave
+           like CrashOnOverflow or RecordOverflow.
+
+        * wtf/CheckedArithmetic.h:
+        (WTF::ConditionalCrashOnOverflow::overflowed):
+        (WTF::ConditionalCrashOnOverflow::shouldCrashOnOverflow const):
+        (WTF::ConditionalCrashOnOverflow::setShouldCrashOnOverflow):
+        (WTF::ConditionalCrashOnOverflow::hasOverflowed const):
+        (WTF::ConditionalCrashOnOverflow::clearOverflow):
+        (WTF::ConditionalCrashOnOverflow::crash):
+        (WTF::RecordOverflow::overflowed):
+        (WTF::Checked::unsafeGet const):
+        * wtf/FastMalloc.cpp:
+        (WTF::tryFastRealloc):
+        * wtf/FastMalloc.h:
+        * wtf/text/StringBuilder.cpp:
+        (WTF::expandedCapacity):
+        (WTF::StringBuilder::reifyString const):
+        (WTF::StringBuilder::resize):
+        (WTF::StringBuilder::allocateBuffer):
+        (WTF::StringBuilder::allocateBufferUpConvert):
+        (WTF::StringBuilder::reallocateBuffer<LChar>):
+        (WTF::StringBuilder::reallocateBuffer<UChar>):
+        (WTF::StringBuilder::reserveCapacity):
+        (WTF::StringBuilder::appendUninitialized):
+        (WTF::StringBuilder::appendUninitializedSlow):
+        (WTF::StringBuilder::append):
+        (WTF::StringBuilder::canShrink const):
+        (WTF::StringBuilder::shrinkToFit):
+        * wtf/text/StringBuilder.h:
+        (WTF::StringBuilder::StringBuilder):
+        (WTF::StringBuilder::didOverflow):
+        (WTF::StringBuilder::hasOverflowed const):
+        (WTF::StringBuilder::crashesOnOverflow const):
+        (WTF::StringBuilder::append):
+        (WTF::StringBuilder::toString):
+        (WTF::StringBuilder::toStringPreserveCapacity const):
+        (WTF::StringBuilder::toAtomicString const):
+        (WTF::StringBuilder::length const):
+        (WTF::StringBuilder::capacity const):
+        (WTF::StringBuilder::operator[] const):
+        (WTF::StringBuilder::swap):
+        (WTF::StringBuilder::getBufferCharacters<UChar>):
+        * wtf/text/StringBuilderJSON.cpp:
+        (WTF::StringBuilder::appendQuotedJSONString):
+        * wtf/text/StringImpl.cpp:
+        (WTF::StringImpl::reallocateInternal):
+        (WTF::StringImpl::reallocate):
+        (WTF::StringImpl::tryReallocate):
+        * wtf/text/StringImpl.h:
+
 2018-10-29  Tim Horton  <timothy_horton@apple.com>
 
         Modernize WebKit nibs and lprojs for localization's sake
index 6298e33..0d6e27b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -70,6 +70,31 @@ enum class CheckedState {
     DidNotOverflow
 };
 
+class ConditionalCrashOnOverflow {
+public:
+    void overflowed()
+    {
+        m_overflowed = true;
+        if (m_shouldCrashOnOverflow)
+            crash();
+    }
+
+    bool shouldCrashOnOverflow() const { return m_shouldCrashOnOverflow; }
+    void setShouldCrashOnOverflow(bool value) { m_shouldCrashOnOverflow = value; }
+
+    bool hasOverflowed() const { return m_overflowed; }
+    void clearOverflow() { m_overflowed = false; }
+
+    static NO_RETURN_DUE_TO_CRASH void crash()
+    {
+        CRASH();
+    }
+
+private:
+    bool m_overflowed { false };
+    bool m_shouldCrashOnOverflow { true };
+};
+
 class CrashOnOverflow {
 public:
     static NO_RETURN_DUE_TO_CRASH void overflowed()
@@ -95,11 +120,6 @@ protected:
     {
     }
 
-    void overflowed()
-    {
-        m_overflowed = true;
-    }
-
     void clearOverflow()
     {
         m_overflowed = false;
@@ -112,6 +132,7 @@ protected:
 
 public:
     bool hasOverflowed() const { return m_overflowed; }
+    void overflowed() { m_overflowed = true; }
 
 private:
     unsigned char m_overflowed;
@@ -206,6 +227,11 @@ template <typename T> struct RemoveChecked {
     static const CleanType DefaultValue = 0;    
 };
 
+template <typename T> struct RemoveChecked<Checked<T, ConditionalCrashOnOverflow>> {
+    using CleanType = typename RemoveChecked<T>::CleanType;
+    static const CleanType DefaultValue = 0;
+};
+
 template <typename T> struct RemoveChecked<Checked<T, CrashOnOverflow>> {
     typedef typename RemoveChecked<T>::CleanType CleanType;
     static const CleanType DefaultValue = 0;
@@ -631,11 +657,12 @@ public:
     }
 
     // Value accessors. unsafeGet() will crash if there's been an overflow.
-    T unsafeGet() const
+    template<typename U = T>
+    U unsafeGet() const
     {
         if (this->hasOverflowed())
             this->crash();
-        return m_value;
+        return static_cast<U>(m_value);
     }
     
     inline CheckedState safeGet(T& value) const WARN_UNUSED_RETURN
@@ -914,6 +941,7 @@ using WTF::CheckedUint32;
 using WTF::CheckedInt64;
 using WTF::CheckedUint64;
 using WTF::CheckedSize;
+using WTF::ConditionalCrashOnOverflow;
 using WTF::CrashOnOverflow;
 using WTF::RecordOverflow;
 using WTF::checkedSum;
index 4566810..cdffc75 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2005, 2007, Google Inc. All rights reserved.
- * Copyright (C) 2005-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2018 Apple Inc. All rights reserved.
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
@@ -221,6 +221,12 @@ void* fastRealloc(void* p, size_t n)
     return result;
 }
 
+TryMallocReturnValue tryFastRealloc(void* p, size_t n)
+{
+    FAIL_IF_EXCEEDS_LIMIT(n);
+    return realloc(p, n);
+}
+
 void releaseFastMallocFreeMemory() { }
 void releaseFastMallocFreeMemoryForThisThread() { }
     
@@ -341,6 +347,12 @@ TryMallocReturnValue tryFastCalloc(size_t numElements, size_t elementSize)
     return tryFastZeroedMalloc(checkedSize.unsafeGet());
 }
     
+TryMallocReturnValue tryFastRealloc(void* object, size_t newSize)
+{
+    FAIL_IF_EXCEEDS_LIMIT(newSize);
+    return bmalloc::api::tryRealloc(object, newSize);
+}
+
 void releaseFastMallocFreeMemoryForThisThread()
 {
     bmalloc::api::scavengeThisThread();
index be6eeab..b796817 100644 (file)
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2005-2009, 2015-2016 Apple Inc. All rights reserved.
+ *  Copyright (C) 2005-2018 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
@@ -53,6 +53,7 @@ WTF_EXPORT_PRIVATE char* fastStrDup(const char*) RETURNS_NONNULL;
 WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastMalloc(size_t);
 WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastZeroedMalloc(size_t);
 WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastCalloc(size_t numElements, size_t elementSize);
+WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastRealloc(void*, size_t);
 
 WTF_EXPORT_PRIVATE void fastFree(void*);
 
index 15e8967..eb3c7b1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
 
 namespace WTF {
 
+static constexpr unsigned maxCapacity = String::MaxLength + 1;
+
 static unsigned expandedCapacity(unsigned capacity, unsigned requiredLength)
 {
     static const unsigned minimumCapacity = 16;
-    return std::max(requiredLength, std::max(minimumCapacity, capacity * 2));
+    return std::max(requiredLength, std::max(minimumCapacity, std::min(capacity * 2, maxCapacity)));
 }
 
 void StringBuilder::reifyString() const
 {
+    ASSERT(!hasOverflowed());
     // Check if the string already exists.
     if (!m_string.isNull()) {
-        ASSERT(m_string.length() == m_length);
+        ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
         return;
     }
 
@@ -55,20 +58,27 @@ void StringBuilder::reifyString() const
     }
 
     // Must be valid in the buffer, take a substring (unless string fills the buffer).
-    ASSERT(m_buffer && m_length <= m_buffer->length());
-    if (m_length == m_buffer->length())
+    ASSERT(m_buffer && m_length.unsafeGet<unsigned>() <= m_buffer->length());
+    if (m_length.unsafeGet<unsigned>() == m_buffer->length())
         m_string = m_buffer.get();
     else
-        m_string = StringImpl::createSubstringSharingImpl(*m_buffer, 0, m_length);
+        m_string = StringImpl::createSubstringSharingImpl(*m_buffer, 0, m_length.unsafeGet());
 }
 
 void StringBuilder::resize(unsigned newSize)
 {
+    if (hasOverflowed())
+        return;
+
     // Check newSize < m_length, hence m_length > 0.
-    ASSERT(newSize <= m_length);
-    if (newSize == m_length)
+    unsigned oldLength = m_length.unsafeGet();
+    ASSERT(newSize <= oldLength);
+    if (newSize == oldLength)
         return;
-    ASSERT(m_length);
+    ASSERT(oldLength);
+
+    m_length = newSize;
+    ASSERT(!hasOverflowed());
 
     // If there is a buffer, we only need to duplicate it if it has more than one ref.
     if (m_buffer) {
@@ -79,16 +89,14 @@ void StringBuilder::resize(unsigned newSize)
             else
                 allocateBuffer(m_buffer->characters16(), m_buffer->length());
         }
-        m_length = newSize;
-        ASSERT(m_buffer->length() >= m_length);
+        ASSERT(hasOverflowed() || m_buffer->length() >= m_length.unsafeGet<unsigned>());
         return;
     }
 
     // Since m_length && !m_buffer, the string must be valid in m_string, and m_string.length() > 0.
     ASSERT(!m_string.isEmpty());
-    ASSERT(m_length == m_string.length());
+    ASSERT(oldLength == m_string.length());
     ASSERT(newSize < m_string.length());
-    m_length = newSize;
     m_string = StringImpl::createSubstringSharingImpl(*m_string.impl(), 0, newSize);
 }
 
@@ -96,10 +104,13 @@ void StringBuilder::resize(unsigned newSize)
 // or m_buffer, neither will be reassigned until the copy has completed).
 void StringBuilder::allocateBuffer(const LChar* currentCharacters, unsigned requiredLength)
 {
+    ASSERT(!hasOverflowed());
     ASSERT(m_is8Bit);
     // Copy the existing data into a new buffer, set result to point to the end of the existing data.
-    auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters8);
-    memcpy(m_bufferCharacters8, currentCharacters, static_cast<size_t>(m_length) * sizeof(LChar)); // This can't overflow.
+    auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters8);
+    if (UNLIKELY(!buffer))
+        return didOverflow();
+    memcpy(m_bufferCharacters8, currentCharacters, static_cast<size_t>(m_length.unsafeGet()) * sizeof(LChar)); // This can't overflow.
     
     // Update the builder state.
     m_buffer = WTFMove(buffer);
@@ -111,10 +122,13 @@ void StringBuilder::allocateBuffer(const LChar* currentCharacters, unsigned requ
 // or m_buffer,  neither will be reassigned until the copy has completed).
 void StringBuilder::allocateBuffer(const UChar* currentCharacters, unsigned requiredLength)
 {
+    ASSERT(!hasOverflowed());
     ASSERT(!m_is8Bit);
     // Copy the existing data into a new buffer, set result to point to the end of the existing data.
-    auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters16);
-    memcpy(m_bufferCharacters16, currentCharacters, static_cast<size_t>(m_length) * sizeof(UChar)); // This can't overflow.
+    auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters16);
+    if (UNLIKELY(!buffer))
+        return didOverflow();
+    memcpy(m_bufferCharacters16, currentCharacters, static_cast<size_t>(m_length.unsafeGet()) * sizeof(UChar)); // This can't overflow.
     
     // Update the builder state.
     m_buffer = WTFMove(buffer);
@@ -126,11 +140,15 @@ void StringBuilder::allocateBuffer(const UChar* currentCharacters, unsigned requ
 // from either m_string or m_buffer, neither will be reassigned until the copy has completed).
 void StringBuilder::allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength)
 {
+    ASSERT(!hasOverflowed());
     ASSERT(m_is8Bit);
-    ASSERT(requiredLength >= m_length);
+    unsigned length = m_length.unsafeGet();
+    ASSERT(requiredLength <= maxCapacity && requiredLength >= length);
     // Copy the existing data into a new buffer, set result to point to the end of the existing data.
-    auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters16);
-    for (unsigned i = 0; i < m_length; ++i)
+    auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters16);
+    if (UNLIKELY(!buffer))
+        return didOverflow(); // Treat a failure to allcoate as an overflow.
+    for (unsigned i = 0; i < length; ++i)
         m_bufferCharacters16[i] = currentCharacters[i];
     
     m_is8Bit = false;
@@ -151,11 +169,14 @@ void StringBuilder::reallocateBuffer<LChar>(unsigned requiredLength)
     ASSERT(m_is8Bit);
     ASSERT(m_buffer->is8Bit());
     
-    if (m_buffer->hasOneRef())
-        m_buffer = StringImpl::reallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters8);
-    else
+    if (m_buffer->hasOneRef()) {
+        auto expectedStringImpl = StringImpl::tryReallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters8);
+        if (UNLIKELY(!expectedStringImpl))
+            return didOverflow();
+        m_buffer = WTFMove(expectedStringImpl.value());
+    } else
         allocateBuffer(m_buffer->characters8(), requiredLength);
-    ASSERT(m_buffer->length() == requiredLength);
+    ASSERT(hasOverflowed() || m_buffer->length() == requiredLength);
 }
 
 template <>
@@ -167,15 +188,21 @@ void StringBuilder::reallocateBuffer<UChar>(unsigned requiredLength)
     
     if (m_buffer->is8Bit())
         allocateBufferUpConvert(m_buffer->characters8(), requiredLength);
-    else if (m_buffer->hasOneRef())
-        m_buffer = StringImpl::reallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters16);
-    else
+    else if (m_buffer->hasOneRef()) {
+        auto expectedStringImpl = StringImpl::tryReallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters16);
+        if (UNLIKELY(!expectedStringImpl))
+            return didOverflow();
+        m_buffer = WTFMove(expectedStringImpl.value());
+    } else
         allocateBuffer(m_buffer->characters16(), requiredLength);
-    ASSERT(m_buffer->length() == requiredLength);
+    ASSERT(hasOverflowed() || m_buffer->length() == requiredLength);
 }
 
 void StringBuilder::reserveCapacity(unsigned newCapacity)
 {
+    if (hasOverflowed())
+        return;
+    ASSERT(newCapacity <= String::MaxLength);
     if (m_buffer) {
         // If there is already a buffer, then grow if necessary.
         if (newCapacity > m_buffer->length()) {
@@ -186,8 +213,9 @@ void StringBuilder::reserveCapacity(unsigned newCapacity)
         }
     } else {
         // Grow the string, if necessary.
-        if (newCapacity > m_length) {
-            if (!m_length) {
+        unsigned length = m_length.unsafeGet();
+        if (newCapacity > length) {
+            if (!length) {
                 LChar* nullPlaceholder = 0;
                 allocateBuffer(nullPlaceholder, newCapacity);
             } else if (m_string.is8Bit())
@@ -196,31 +224,34 @@ void StringBuilder::reserveCapacity(unsigned newCapacity)
                 allocateBuffer(m_string.characters16(), newCapacity);
         }
     }
-    ASSERT(!newCapacity || m_buffer->length() >= newCapacity);
+    ASSERT(hasOverflowed() || !newCapacity || m_buffer->length() >= newCapacity);
 }
 
 // Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
 // return a pointer to the newly allocated storage.
+// Returns nullptr if the size of the new builder would have overflowed
 template <typename CharType>
 ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
 {
     ASSERT(length);
 
     // Calculate the new size of the builder after appending.
-    unsigned requiredLength = length + m_length;
-    if (requiredLength < length)
-        CRASH();
+    CheckedInt32 requiredLength = m_length + length;
+    if (requiredLength.hasOverflowed()) {
+        didOverflow();
+        return nullptr;
+    }
 
-    if ((m_buffer) && (requiredLength <= m_buffer->length())) {
+    if (m_buffer && (requiredLength.unsafeGet<unsigned>() <= m_buffer->length())) {
         // If the buffer is valid it must be at least as long as the current builder contents!
-        ASSERT(m_buffer->length() >= m_length);
-        unsigned currentLength = m_length;
+        ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
+        unsigned currentLength = m_length.unsafeGet();
         m_string = String();
         m_length = requiredLength;
         return getBufferCharacters<CharType>() + currentLength;
     }
     
-    return appendUninitializedSlow<CharType>(requiredLength);
+    return appendUninitializedSlow<CharType>(requiredLength.unsafeGet());
 }
 
 // Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
@@ -228,27 +259,31 @@ ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
 template <typename CharType>
 CharType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
 {
+    ASSERT(!hasOverflowed());
     ASSERT(requiredLength);
 
     if (m_buffer) {
         // If the buffer is valid it must be at least as long as the current builder contents!
-        ASSERT(m_buffer->length() >= m_length);
+        ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
         
         reallocateBuffer<CharType>(expandedCapacity(capacity(), requiredLength));
     } else {
-        ASSERT(m_string.length() == m_length);
+        ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
         allocateBuffer(m_length ? m_string.characters<CharType>() : 0, expandedCapacity(capacity(), requiredLength));
     }
-    
-    CharType* result = getBufferCharacters<CharType>() + m_length;
+    if (UNLIKELY(hasOverflowed()))
+        return nullptr;
+
+    CharType* result = getBufferCharacters<CharType>() + m_length.unsafeGet();
     m_length = requiredLength;
-    ASSERT(m_buffer->length() >= m_length);
+    ASSERT(!hasOverflowed());
+    ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
     return result;
 }
 
 void StringBuilder::append(const UChar* characters, unsigned length)
 {
-    if (!length)
+    if (!length || hasOverflowed())
         return;
 
     ASSERT(characters);
@@ -257,40 +292,50 @@ void StringBuilder::append(const UChar* characters, unsigned length)
         if (length == 1 && !(*characters & ~0xff)) {
             // Append as 8 bit character
             LChar lChar = static_cast<LChar>(*characters);
-            append(&lChar, 1);
-            return;
+            return append(&lChar, 1);
         }
 
         // Calculate the new size of the builder after appending.
-        unsigned requiredLength = length + m_length;
-        if (requiredLength < length)
-            CRASH();
+        CheckedInt32 requiredLength = m_length + length;
+        if (requiredLength.hasOverflowed())
+            return didOverflow();
         
         if (m_buffer) {
             // If the buffer is valid it must be at least as long as the current builder contents!
-            ASSERT(m_buffer->length() >= m_length);
-            
-            allocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength));
+            ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
+            allocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength.unsafeGet()));
         } else {
-            ASSERT(m_string.length() == m_length);
-            allocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength));
+            ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
+            allocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength.unsafeGet()));
         }
+        if (UNLIKELY(hasOverflowed()))
+            return;
 
-        memcpy(m_bufferCharacters16 + m_length, characters, static_cast<size_t>(length) * sizeof(UChar));
+        memcpy(m_bufferCharacters16 + m_length.unsafeGet<unsigned>(), characters, static_cast<size_t>(length) * sizeof(UChar));
         m_length = requiredLength;
-    } else
-        memcpy(appendUninitialized<UChar>(length), characters, static_cast<size_t>(length) * sizeof(UChar));
-    ASSERT(m_buffer->length() >= m_length);
+    } else {
+        UChar* dest = appendUninitialized<UChar>(length);
+        if (!dest)
+            return;
+        memcpy(dest, characters, static_cast<size_t>(length) * sizeof(UChar));
+    }
+    ASSERT(!hasOverflowed());
+    ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
 }
 
 void StringBuilder::append(const LChar* characters, unsigned length)
 {
-    if (!length)
+    if (!length || hasOverflowed())
         return;
+
     ASSERT(characters);
 
     if (m_is8Bit) {
         LChar* dest = appendUninitialized<LChar>(length);
+        if (!dest) {
+            ASSERT(hasOverflowed());
+            return;
+        }
         if (length > 8)
             memcpy(dest, characters, static_cast<size_t>(length) * sizeof(LChar));
         else {
@@ -300,6 +345,10 @@ void StringBuilder::append(const LChar* characters, unsigned length)
         }
     } else {
         UChar* dest = appendUninitialized<UChar>(length);
+        if (!dest) {
+            ASSERT(hasOverflowed());
+            return;
+        }
         const LChar* end = characters + length;
         while (characters < end)
             *(dest++) = *(characters++);
@@ -370,17 +419,21 @@ void StringBuilder::appendFixedWidthNumber(double number, unsigned decimalPlaces
 
 bool StringBuilder::canShrink() const
 {
+    if (hasOverflowed())
+        return false;
     // Only shrink the buffer if it's less than 80% full. Need to tune this heuristic!
-    return m_buffer && m_buffer->length() > (m_length + (m_length >> 2));
+    unsigned length = m_length.unsafeGet();
+    return m_buffer && m_buffer->length() > (length + (length >> 2));
 }
 
 void StringBuilder::shrinkToFit()
 {
     if (canShrink()) {
         if (m_is8Bit)
-            reallocateBuffer<LChar>(m_length);
+            reallocateBuffer<LChar>(m_length.unsafeGet());
         else
-            reallocateBuffer<UChar>(m_length);
+            reallocateBuffer<UChar>(m_length.unsafeGet());
+        ASSERT(!hasOverflowed());
         m_string = WTFMove(m_buffer);
     }
 }
index 43cfb8d..3cbf51a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2010, 2012-2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <wtf/CheckedArithmetic.h>
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/IntegerToStringConversion.h>
 #include <wtf/text/StringView.h>
 
 namespace WTF {
 
+// StringBuilder currently uses a Checked<int32_t, ConditionalCrashOnOverflow> for m_length.
+// Ideally, we would want to make StringBuilder a template with an OverflowHandler parameter, and
+// m_length can be instantiated based on that OverflowHandler instead. However, currently, we're
+// not able to get clang to export explicitly instantiated template methods (which would be needed
+// if we templatize StringBuilder). As a workaround, we use the ConditionalCrashOnOverflow handler
+// instead to do a runtime check on whether it should crash on overflows or not.
+//
+// When clang is able to export explicitly instantiated template methods, we can templatize
+// StringBuilder and do away with ConditionalCrashOnOverflow.
+// See https://bugs.webkit.org/show_bug.cgi?id=191050.
+
 class StringBuilder {
     // Disallow copying since it's expensive and we don't want code to do it by accident.
     WTF_MAKE_NONCOPYABLE(StringBuilder);
 
 public:
-    StringBuilder()
+    enum class OverflowHandler {
+        CrashOnOverflow,
+        RecordOverflow
+    };
+
+    StringBuilder(OverflowHandler handler = OverflowHandler::CrashOnOverflow)
         : m_bufferCharacters8(nullptr)
     {
+        m_length.setShouldCrashOnOverflow(handler == OverflowHandler::CrashOnOverflow);
     }
     StringBuilder(StringBuilder&&) = default;
     StringBuilder& operator=(StringBuilder&&) = default;
 
+    ALWAYS_INLINE void didOverflow() { m_length.overflowed(); }
+    ALWAYS_INLINE bool hasOverflowed() const { return m_length.hasOverflowed(); }
+    ALWAYS_INLINE bool crashesOnOverflow() const { return m_length.shouldCrashOnOverflow(); }
+
     WTF_EXPORT_PRIVATE void append(const UChar*, unsigned);
     WTF_EXPORT_PRIVATE void append(const LChar*, unsigned);
 
@@ -57,6 +79,9 @@ public:
 
     void append(const String& string)
     {
+        if (hasOverflowed())
+            return;
+
         if (!string.length())
             return;
 
@@ -77,6 +102,11 @@ public:
 
     void append(const StringBuilder& other)
     {
+        if (hasOverflowed())
+            return;
+        if (other.hasOverflowed())
+            return didOverflow();
+
         if (!other.m_length)
             return;
 
@@ -89,9 +119,9 @@ public:
         }
 
         if (other.is8Bit())
-            append(other.characters8(), other.m_length);
+            append(other.characters8(), other.m_length.unsafeGet());
         else
-            append(other.characters16(), other.m_length);
+            append(other.characters16(), other.m_length.unsafeGet());
     }
 
     void append(StringView stringView)
@@ -131,14 +161,19 @@ public:
 
     void append(UChar c)
     {
-        if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
+        if (hasOverflowed())
+            return;
+        unsigned length = m_length.unsafeGet<unsigned>();
+        if (m_buffer && length < m_buffer->length() && m_string.isNull()) {
             if (!m_is8Bit) {
-                m_bufferCharacters16[m_length++] = c;
+                m_bufferCharacters16[length] = c;
+                m_length++;
                 return;
             }
 
             if (!(c & ~0xff)) {
-                m_bufferCharacters8[m_length++] = static_cast<LChar>(c);
+                m_bufferCharacters8[length] = static_cast<LChar>(c);
+                m_length++;
                 return;
             }
         }
@@ -147,11 +182,15 @@ public:
 
     void append(LChar c)
     {
-        if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
+        if (hasOverflowed())
+            return;
+        unsigned length = m_length.unsafeGet<unsigned>();
+        if (m_buffer && length < m_buffer->length() && m_string.isNull()) {
             if (m_is8Bit)
-                m_bufferCharacters8[m_length++] = c;
+                m_bufferCharacters8[length] = c;
             else
-                m_bufferCharacters16[m_length++] = c;
+                m_bufferCharacters16[length] = c;
+            m_length++;
         } else
             append(&c, 1);
     }
@@ -171,7 +210,7 @@ public:
         append(U16_TRAIL(c));
     }
 
-    WTF_EXPORT_PRIVATE bool appendQuotedJSONString(const String&);
+    WTF_EXPORT_PRIVATE void appendQuotedJSONString(const String&);
 
     template<unsigned characterCount>
     ALWAYS_INLINE void appendLiteral(const char (&characters)[characterCount]) { append(characters, characterCount - 1); }
@@ -188,6 +227,7 @@ public:
 
     String toString()
     {
+        RELEASE_ASSERT(!hasOverflowed());
         shrinkToFit();
         if (m_string.isNull())
             reifyString();
@@ -196,6 +236,7 @@ public:
 
     const String& toStringPreserveCapacity() const
     {
+        RELEASE_ASSERT(!hasOverflowed());
         if (m_string.isNull())
             reifyString();
         return m_string;
@@ -203,6 +244,7 @@ public:
 
     AtomicString toAtomicString() const
     {
+        RELEASE_ASSERT(!hasOverflowed());
         if (!m_length)
             return emptyAtom();
 
@@ -217,12 +259,13 @@ public:
             return AtomicString(m_string);
 
         ASSERT(m_buffer);
-        return AtomicString(m_buffer.get(), 0, m_length);
+        return AtomicString(m_buffer.get(), 0, m_length.unsafeGet());
     }
 
     unsigned length() const
     {
-        return m_length;
+        RELEASE_ASSERT(!hasOverflowed());
+        return m_length.unsafeGet();
     }
 
     bool isEmpty() const { return !m_length; }
@@ -231,7 +274,8 @@ public:
 
     unsigned capacity() const
     {
-        return m_buffer ? m_buffer->length() : m_length;
+        RELEASE_ASSERT(!hasOverflowed());
+        return m_buffer ? m_buffer->length() : m_length.unsafeGet();
     }
 
     WTF_EXPORT_PRIVATE void resize(unsigned newSize);
@@ -242,7 +286,7 @@ public:
 
     UChar operator[](unsigned i) const
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(i < m_length);
+        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!hasOverflowed() && i < m_length.unsafeGet<unsigned>());
         if (m_is8Bit)
             return characters8()[i];
         return characters16()[i];
@@ -288,7 +332,7 @@ public:
         m_buffer.swap(stringBuilder.m_buffer);
         std::swap(m_is8Bit, stringBuilder.m_is8Bit);
         std::swap(m_bufferCharacters8, stringBuilder.m_bufferCharacters8);
-        ASSERT(!m_buffer || m_buffer->length() >= m_length);
+        ASSERT(!m_buffer || hasOverflowed() || m_buffer->length() >= m_length.unsafeGet<unsigned>());
     }
 
 private:
@@ -311,7 +355,8 @@ private:
         LChar* m_bufferCharacters8;
         UChar* m_bufferCharacters16;
     };
-    unsigned m_length { 0 };
+    static_assert(String::MaxLength == std::numeric_limits<int32_t>::max(), "");
+    Checked<int32_t, ConditionalCrashOnOverflow> m_length;
     bool m_is8Bit { true };
 };
 
@@ -327,7 +372,7 @@ ALWAYS_INLINE UChar* StringBuilder::getBufferCharacters<UChar>()
 {
     ASSERT(!m_is8Bit);
     return m_bufferCharacters16;
-}    
+}
 
 template <typename CharType>
 bool equal(const StringBuilder& s, const CharType* buffer, unsigned length)
index c3a40b8..f382c1b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  * Copyright (C) 2017 Yusuke Suzuki <utatane.tea@gmail.com>. All rights reserved.
  * Copyright (C) 2017 Mozilla Foundation. All rights reserved.
@@ -74,8 +74,10 @@ ALWAYS_INLINE static void appendQuotedJSONStringInternal(OutputCharacterType*& o
     }
 }
 
-bool StringBuilder::appendQuotedJSONString(const String& string)
+void StringBuilder::appendQuotedJSONString(const String& string)
 {
+    if (hasOverflowed())
+        return;
     // Make sure we have enough buffer space to append this string without having
     // to worry about reallocating in the middle.
     // The 2 is for the '"' quotes on each end.
@@ -85,31 +87,33 @@ bool StringBuilder::appendQuotedJSONString(const String& string)
     maximumCapacityRequired += 2 + stringLength * 6;
     unsigned allocationSize;
     if (CheckedState::DidOverflow == maximumCapacityRequired.safeGet(allocationSize))
-        return false;
+        return didOverflow();
     // This max() is here to allow us to allocate sizes between the range [2^31, 2^32 - 2] because roundUpToPowerOfTwo(1<<31 + some int smaller than 1<<31) == 0.
     // FIXME: roundUpToPowerOfTwo should take Checked<unsigned> and abort if it fails to round up.
     // https://bugs.webkit.org/show_bug.cgi?id=176086
     allocationSize = std::max(allocationSize, roundUpToPowerOfTwo(allocationSize));
 
     // Allocating this much will definitely fail.
-    if (allocationSize >= 0x80000000)
-        return false;
+    if (allocationSize > String::MaxLength)
+        return didOverflow();
 
     if (is8Bit() && !string.is8Bit())
         allocateBufferUpConvert(m_bufferCharacters8, allocationSize);
     else
         reserveCapacity(allocationSize);
+    if (UNLIKELY(hasOverflowed()))
+        return;
     ASSERT(m_buffer->length() >= allocationSize);
 
     if (is8Bit()) {
         ASSERT(string.is8Bit());
-        LChar* output = m_bufferCharacters8 + m_length;
+        LChar* output = m_bufferCharacters8 + m_length.unsafeGet<unsigned>();
         *output++ = '"';
         appendQuotedJSONStringInternal(output, string.characters8(), string.length());
         *output++ = '"';
         m_length = output - m_bufferCharacters8;
     } else {
-        UChar* output = m_bufferCharacters16 + m_length;
+        UChar* output = m_bufferCharacters16 + m_length.unsafeGet<unsigned>();
         *output++ = '"';
         if (string.is8Bit())
             appendQuotedJSONStringInternal(output, string.characters8(), string.length());
@@ -118,8 +122,8 @@ bool StringBuilder::appendQuotedJSONString(const String& string)
         *output++ = '"';
         m_length = output - m_bufferCharacters16;
     }
-    ASSERT(m_buffer->length() >= m_length);
-    return true;
+    ASSERT(!hasOverflowed());
+    ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
 }
 
 } // namespace WTF
index 5392795..437160b 100644 (file)
@@ -2,7 +2,7 @@
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller ( mueller@kde.org )
- * Copyright (C) 2003-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
  *
  * This library is free software; you can redistribute it and/or
@@ -211,22 +211,24 @@ Ref<StringImpl> StringImpl::createUninitialized(unsigned length, UChar*& data)
     return createUninitializedInternal(length, data);
 }
 
-template<typename CharacterType> inline Ref<StringImpl> StringImpl::reallocateInternal(Ref<StringImpl>&& originalString, unsigned length, CharacterType*& data)
+template<typename CharacterType> inline Expected<Ref<StringImpl>, UTF8ConversionError> StringImpl::reallocateInternal(Ref<StringImpl>&& originalString, unsigned length, CharacterType*& data)
 {
     ASSERT(originalString->hasOneRef());
     ASSERT(originalString->bufferOwnership() == BufferInternal);
 
     if (!length) {
         data = 0;
-        return *empty();
+        return Ref<StringImpl>(*empty());
     }
 
     // Same as createUninitialized() except here we use fastRealloc.
     if (length > ((std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) / sizeof(CharacterType)))
-        CRASH();
+        return makeUnexpected(UTF8ConversionError::OutOfMemory);
 
     originalString->~StringImpl();
-    auto* string = static_cast<StringImpl*>(fastRealloc(&originalString.leakRef(), allocationSize<CharacterType>(length)));
+    StringImpl* string;
+    if (!tryFastRealloc(&originalString.leakRef(), allocationSize<CharacterType>(length)).getValue(string))
+        return makeUnexpected(UTF8ConversionError::OutOfMemory);
 
     data = string->tailPointer<CharacterType>();
     return constructInternal<CharacterType>(*string, length);
@@ -234,11 +236,25 @@ template<typename CharacterType> inline Ref<StringImpl> StringImpl::reallocateIn
 
 Ref<StringImpl> StringImpl::reallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data)
 {
+    auto expectedStringImpl = tryReallocate(WTFMove(originalString), length, data);
+    RELEASE_ASSERT(expectedStringImpl);
+    return WTFMove(expectedStringImpl.value());
+}
+
+Ref<StringImpl> StringImpl::reallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data)
+{
+    auto expectedStringImpl = tryReallocate(WTFMove(originalString), length, data);
+    RELEASE_ASSERT(expectedStringImpl);
+    return WTFMove(expectedStringImpl.value());
+}
+
+Expected<Ref<StringImpl>, UTF8ConversionError> StringImpl::tryReallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data)
+{
     ASSERT(originalString->is8Bit());
     return reallocateInternal(WTFMove(originalString), length, data);
 }
 
-Ref<StringImpl> StringImpl::reallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data)
+Expected<Ref<StringImpl>, UTF8ConversionError> StringImpl::tryReallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data)
 {
     ASSERT(!originalString->is8Bit());
     return reallocateInternal(WTFMove(originalString), length, data);
index bb1c034..14a66ed 100644 (file)
@@ -254,6 +254,8 @@ public:
     // the originalString can't be used after this function.
     static Ref<StringImpl> reallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data);
     static Ref<StringImpl> reallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data);
+    static Expected<Ref<StringImpl>, UTF8ConversionError> tryReallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data);
+    static Expected<Ref<StringImpl>, UTF8ConversionError> tryReallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data);
 
     static unsigned flagsOffset() { return OBJECT_OFFSETOF(StringImpl, m_hashAndFlags); }
     static unsigned flagIs8Bit() { return s_hashFlag8BitBuffer; }
@@ -510,7 +512,7 @@ private:
     template<typename CharacterType> static Ref<StringImpl> constructInternal(StringImpl&, unsigned);
     template<typename CharacterType> static Ref<StringImpl> createUninitializedInternal(unsigned, CharacterType*&);
     template<typename CharacterType> static Ref<StringImpl> createUninitializedInternalNonEmpty(unsigned, CharacterType*&);
-    template<typename CharacterType> static Ref<StringImpl> reallocateInternal(Ref<StringImpl>&&, unsigned, CharacterType*&);
+    template<typename CharacterType> static Expected<Ref<StringImpl>, UTF8ConversionError> reallocateInternal(Ref<StringImpl>&&, unsigned, CharacterType*&);
     template<typename CharacterType> static Ref<StringImpl> createInternal(const CharacterType*, unsigned);
     WTF_EXPORT_PRIVATE NEVER_INLINE unsigned hashSlowCase() const;
 
index 42f30c8..9b5602f 100644 (file)
@@ -1,3 +1,24 @@
+2018-10-29  Mark Lam  <mark.lam@apple.com>
+
+        Correctly detect string overflow when using the 'Function' constructor.
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        <rdar://problem/36320331>
+
+        Reviewed by Saam Barati.
+
+        * bmalloc/Allocator.cpp:
+        (bmalloc::Allocator::reallocate):
+        (bmalloc::Allocator::tryReallocate):
+        (bmalloc::Allocator::reallocateImpl):
+        * bmalloc/Allocator.h:
+        * bmalloc/Cache.h:
+        (bmalloc::Cache::tryReallocate):
+        * bmalloc/DebugHeap.cpp:
+        (bmalloc::DebugHeap::realloc):
+        * bmalloc/DebugHeap.h:
+        * bmalloc/bmalloc.h:
+        (bmalloc::api::tryRealloc):
+
 2018-10-25  Ross Kirsling  <ross.kirsling@sony.com>
 
         Cleanup: inline constexpr is redundant as constexpr implies inline
index f2d93b7..4fbdb09 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -95,8 +95,20 @@ void* Allocator::allocateImpl(size_t alignment, size_t size, bool crashOnFailure
 
 void* Allocator::reallocate(void* object, size_t newSize)
 {
+    bool crashOnFailure = true;
+    return reallocateImpl(object, newSize, crashOnFailure);
+}
+
+void* Allocator::tryReallocate(void* object, size_t newSize)
+{
+    bool crashOnFailure = false;
+    return reallocateImpl(object, newSize, crashOnFailure);
+}
+
+void* Allocator::reallocateImpl(void* object, size_t newSize, bool crashOnFailure)
+{
     if (m_debugHeap)
-        return m_debugHeap->realloc(object, newSize);
+        return m_debugHeap->realloc(object, newSize, crashOnFailure);
 
     size_t oldSize = 0;
     switch (objectType(m_heap.kind(), object)) {
@@ -121,7 +133,14 @@ void* Allocator::reallocate(void* object, size_t newSize)
     }
     }
 
-    void* result = allocate(newSize);
+    void* result = nullptr;
+    if (crashOnFailure)
+        result = allocate(newSize);
+    else {
+        result = tryAllocate(newSize);
+        if (!result)
+            return nullptr;
+    }
     size_t copySize = std::min(oldSize, newSize);
     memcpy(result, object, copySize);
     m_deallocator.deallocate(object);
index d26ce92..7c894de 100644 (file)
@@ -47,13 +47,15 @@ public:
     void* allocate(size_t);
     void* tryAllocate(size_t alignment, size_t);
     void* allocate(size_t alignment, size_t);
+    void* tryReallocate(void*, size_t);
     void* reallocate(void*, size_t);
 
     void scavenge();
 
 private:
     void* allocateImpl(size_t alignment, size_t, bool crashOnFailure);
-    
+    void* reallocateImpl(void*, size_t, bool crashOnFailure);
+
     bool allocateFastCase(size_t, void*&);
     BEXPORT void* allocateSlowCase(size_t);
     
index c414ec8..fde7e09 100644 (file)
@@ -43,6 +43,7 @@ public:
     static void* tryAllocate(HeapKind, size_t alignment, size_t);
     static void* allocate(HeapKind, size_t alignment, size_t);
     static void deallocate(HeapKind, void*);
+    static void* tryReallocate(HeapKind, void*, size_t);
     static void* reallocate(HeapKind, void*, size_t);
 
     static void scavenge(HeapKind);
@@ -103,6 +104,14 @@ inline void Cache::deallocate(HeapKind heapKind, void* object)
     return caches->at(mapToActiveHeapKindAfterEnsuringGigacage(heapKind)).deallocator().deallocate(object);
 }
 
+inline void* Cache::tryReallocate(HeapKind heapKind, void* object, size_t newSize)
+{
+    PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
+    if (!caches)
+        return reallocateSlowCaseNullCache(heapKind, object, newSize);
+    return caches->at(mapToActiveHeapKindAfterEnsuringGigacage(heapKind)).allocator().tryReallocate(object, newSize);
+}
+
 inline void* Cache::reallocate(HeapKind heapKind, void* object, size_t newSize)
 {
     PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
index da730a6..f21f5a5 100644 (file)
@@ -59,10 +59,10 @@ void* DebugHeap::memalign(size_t alignment, size_t size, bool crashOnFailure)
     return result;
 }
 
-void* DebugHeap::realloc(void* object, size_t size)
+void* DebugHeap::realloc(void* object, size_t size, bool crashOnFailure)
 {
     void* result = malloc_zone_realloc(m_zone, object, size);
-    if (!result)
+    if (!result && crashOnFailure)
         BCRASH();
     return result;
 }
@@ -98,10 +98,10 @@ void* DebugHeap::memalign(size_t alignment, size_t size, bool crashOnFailure)
     return result;
 }
 
-void* DebugHeap::realloc(void* object, size_t size)
+void* DebugHeap::realloc(void* object, size_t size, bool crashOnFailure)
 {
     void* result = ::realloc(object, size);
-    if (!result)
+    if (!result && crashOnFailure)
         BCRASH();
     return result;
 }
index 0b16c3f..219f837 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -41,7 +41,7 @@ public:
     
     void* malloc(size_t);
     void* memalign(size_t alignment, size_t, bool crashOnFailure);
-    void* realloc(void*, size_t);
+    void* realloc(void*, size_t, bool crashOnFailure);
     void free(void*);
     
     void* memalignLarge(size_t alignment, size_t);
index e46d435..775ac0a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -63,6 +63,12 @@ inline void* memalign(size_t alignment, size_t size, HeapKind kind = HeapKind::P
     return Cache::allocate(kind, alignment, size);
 }
 
+// Returns null on failure.
+inline void* tryRealloc(void* object, size_t newSize, HeapKind kind = HeapKind::Primary)
+{
+    return Cache::tryReallocate(kind, object, newSize);
+}
+
 // Crashes on failure.
 inline void* realloc(void* object, size_t newSize, HeapKind kind = HeapKind::Primary)
 {