Diff
Modified: trunk/JSTests/ChangeLog (231196 => 231197)
--- trunk/JSTests/ChangeLog 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/JSTests/ChangeLog 2018-05-01 16:01:25 UTC (rev 231197)
@@ -1,5 +1,18 @@
2018-05-01 Robin Morisset <[email protected]>
+ Correctly detect string overflow when using the 'Function' constructor
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Filip Pizlo.
+
+ I put this regression test in the 'slowMicrobenchmarks' directory because it takes nearly 30s to run, and I am not sure where else to put it.
+
+ * slowMicrobenchmarks/function-constructor-with-huge-strings.js: Added.
+ (catch):
+
+2018-05-01 Robin Morisset <[email protected]>
+
IntlObject.cpp::removeUnicodeLocaleExtension() should not touch locales that end in '-u'
https://bugs.webkit.org/show_bug.cgi?id=185162
Added: trunk/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js (0 => 231197)
--- trunk/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js (rev 0)
+++ trunk/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js 2018-05-01 16:01:25 UTC (rev 231197)
@@ -0,0 +1,13 @@
+var hugeString = "x";
+for (i = 0; i < 25; ++i) {
+ hugeString += hugeString;
+}
+
+var weird = 'Â';
+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) {}
Modified: trunk/Source/_javascript_Core/ChangeLog (231196 => 231197)
--- trunk/Source/_javascript_Core/ChangeLog 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-05-01 16:01:25 UTC (rev 231197)
@@ -1,5 +1,25 @@
2018-05-01 Robin Morisset <[email protected]>
+ Correctly detect string overflow when using the 'Function' constructor
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Filip Pizlo.
+
+ The 'Function' constructor creates a string containing the source code of the new function through repeated string concatenation.
+ Because there was no way for the string concatenation routines in WTF to return an error, they just crashed in that case.
+
+ I added new tryAppend methods alongside the old append methods, that return a boolean (true means success, false means an overflow happened).
+ In this way, it becomes possible for the Function constructor to just throw a proper JS exception when asked to create a string > 4GB.
+ I made new methods instead of just adapting the existing ones (and reverted such a change on appendQuotedJSONString) so that callers that rely on the old behaviour (a hard CRASH() on overflow) don't silently start failing.
+
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ * runtime/JSONObject.cpp:
+ (JSC::Stringifier::appendStringifiedValue):
+
+2018-05-01 Robin Morisset <[email protected]>
+
IntlObject.cpp::removeUnicodeLocaleExtension() should not touch locales that end in '-u'
https://bugs.webkit.org/show_bug.cgi?id=185162
Modified: trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp (231196 => 231197)
--- trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2018-05-01 16:01:25 UTC (rev 231197)
@@ -125,20 +125,25 @@
program = makeString("{", prefix, functionName.string(), "() {\n", body, "\n}}");
} else {
StringBuilder builder;
- builder.append('{');
- builder.append(prefix);
- builder.append(functionName.string());
- builder.append('(');
+ bool success = true;
+ success &= builder.tryAppend('{');
+ success &= builder.tryAppend(prefix);
+ success &= builder.tryAppend(functionName.string());
+ success &= builder.tryAppend('(');
StringBuilder parameterBuilder;
auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(exec);
RETURN_IF_EXCEPTION(scope, nullptr);
- parameterBuilder.append(viewWithString.view);
+ success &= parameterBuilder.tryAppend(viewWithString.view);
for (size_t i = 1; i < args.size() - 1; i++) {
- parameterBuilder.appendLiteral(", ");
+ success &= parameterBuilder.tryAppendLiteral(", ");
auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(exec);
RETURN_IF_EXCEPTION(scope, nullptr);
- parameterBuilder.append(viewWithString.view);
+ success &= parameterBuilder.tryAppend(viewWithString.view);
}
+ if (!success) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
{
// The spec mandates that the parameters parse as a valid parameter list
@@ -153,14 +158,18 @@
}
}
- builder.append(parameterBuilder);
- builder.appendLiteral(") {\n");
+ success &= builder.tryAppend(parameterBuilder);
+ success &= builder.tryAppendLiteral(") {\n");
auto body = args.at(args.size() - 1).toWTFString(exec);
RETURN_IF_EXCEPTION(scope, nullptr);
checkBody(body);
RETURN_IF_EXCEPTION(scope, nullptr);
- builder.append(body);
- builder.appendLiteral("\n}}");
+ success &= builder.tryAppend(body);
+ success &= builder.tryAppendLiteral("\n}}");
+ if (!success) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
program = builder.toString();
}
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (231196 => 231197)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-05-01 16:01:25 UTC (rev 231197)
@@ -357,7 +357,7 @@
if (value.isString()) {
const String& string = asString(value)->value(m_exec);
RETURN_IF_EXCEPTION(scope, StringifyFailed);
- if (builder.appendQuotedJSONString(string))
+ if (builder.tryAppendQuotedJSONString(string))
return StringifySucceeded;
throwOutOfMemoryError(m_exec, scope);
return StringifyFailed;
Modified: trunk/Source/WTF/ChangeLog (231196 => 231197)
--- trunk/Source/WTF/ChangeLog 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/WTF/ChangeLog 2018-05-01 16:01:25 UTC (rev 231197)
@@ -1,3 +1,28 @@
+2018-05-01 Robin Morisset <[email protected]>
+
+ Correctly detect string overflow when using the 'Function' constructor
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Filip Pizlo.
+
+ I added new tryAppend methods alongside the old append methods in StringBuilder, that return a boolean (true means success, false means an overflow happened).
+ I made new methods instead of just adapting the existing ones (and reverted such a change on appendQuotedJSONString) so that callers that rely on the old behaviour (a hard CRASH() on overflow) don't silently start failing.
+
+ * wtf/text/StringBuilder.cpp:
+ (WTF::StringBuilder::allocateBufferUpConvert):
+ (WTF::StringBuilder::tryAllocateBufferUpConvert):
+ (WTF::StringBuilder::appendUninitialized):
+ (WTF::StringBuilder::append):
+ (WTF::StringBuilder::tryAppend):
+ * wtf/text/StringBuilder.h:
+ (WTF::StringBuilder::tryAppend):
+ (WTF::StringBuilder::append):
+ (WTF::StringBuilder::tryAppendLiteral):
+ * wtf/text/StringBuilderJSON.cpp:
+ (WTF::StringBuilder::appendQuotedJSONString):
+ (WTF::StringBuilder::tryAppendQuotedJSONString):
+
2018-04-30 Mark Lam <[email protected]>
Apply PtrTags to the MetaAllocator and friends.
Modified: trunk/Source/WTF/wtf/text/StringBuilder.cpp (231196 => 231197)
--- trunk/Source/WTF/wtf/text/StringBuilder.cpp 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/WTF/wtf/text/StringBuilder.cpp 2018-05-01 16:01:25 UTC (rev 231197)
@@ -126,10 +126,17 @@
// from either m_string or m_buffer, neither will be reassigned until the copy has completed).
void StringBuilder::allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength)
{
+ if (!tryAllocateBufferUpConvert(currentCharacters, requiredLength))
+ CRASH();
+}
+bool StringBuilder::tryAllocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength)
+{
ASSERT(m_is8Bit);
ASSERT(requiredLength >= m_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);
+ auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters16);
+ if (!buffer)
+ return false;
for (unsigned i = 0; i < m_length; ++i)
m_bufferCharacters16[i] = currentCharacters[i];
@@ -139,6 +146,7 @@
m_buffer = WTFMove(buffer);
m_string = String();
ASSERT(m_buffer->length() == requiredLength);
+ return true;
}
template <>
@@ -201,6 +209,7 @@
// 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)
{
@@ -209,7 +218,7 @@
// Calculate the new size of the builder after appending.
unsigned requiredLength = length + m_length;
if (requiredLength < length)
- CRASH();
+ return nullptr;
if ((m_buffer) && (requiredLength <= m_buffer->length())) {
// If the buffer is valid it must be at least as long as the current builder contents!
@@ -248,8 +257,14 @@
void StringBuilder::append(const UChar* characters, unsigned length)
{
+ if (!tryAppend(characters, length))
+ CRASH();
+}
+
+bool StringBuilder::tryAppend(const UChar* characters, unsigned length)
+{
if (!length)
- return;
+ return true;
ASSERT(characters);
@@ -257,40 +272,54 @@
if (length == 1 && !(*characters & ~0xff)) {
// Append as 8 bit character
LChar lChar = static_cast<LChar>(*characters);
- append(&lChar, 1);
- return;
+ return tryAppend(&lChar, 1);
}
// Calculate the new size of the builder after appending.
unsigned requiredLength = length + m_length;
if (requiredLength < length)
- CRASH();
+ return false;
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));
+ if (!tryAllocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength)))
+ return false;
} else {
ASSERT(m_string.length() == m_length);
- allocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength));
+ if (!tryAllocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength)))
+ return false;
}
memcpy(m_bufferCharacters16 + m_length, characters, static_cast<size_t>(length) * sizeof(UChar));
m_length = requiredLength;
- } else
- memcpy(appendUninitialized<UChar>(length), characters, static_cast<size_t>(length) * sizeof(UChar));
+ } else {
+ UChar* dest = appendUninitialized<UChar>(length);
+ if (!dest)
+ return false;
+ memcpy(dest, characters, static_cast<size_t>(length) * sizeof(UChar));
+ }
ASSERT(m_buffer->length() >= m_length);
+ return true;
}
void StringBuilder::append(const LChar* characters, unsigned length)
{
+ if (!tryAppend(characters, length))
+ CRASH();
+}
+
+bool StringBuilder::tryAppend(const LChar* characters, unsigned length)
+{
if (!length)
- return;
+ return true;
+
ASSERT(characters);
if (m_is8Bit) {
LChar* dest = appendUninitialized<LChar>(length);
+ if (!dest)
+ return false;
if (length > 8)
memcpy(dest, characters, static_cast<size_t>(length) * sizeof(LChar));
else {
@@ -300,10 +329,13 @@
}
} else {
UChar* dest = appendUninitialized<UChar>(length);
+ if (!dest)
+ return false;
const LChar* end = characters + length;
while (characters < end)
*(dest++) = *(characters++);
}
+ return true;
}
#if USE(CF)
Modified: trunk/Source/WTF/wtf/text/StringBuilder.h (231196 => 231197)
--- trunk/Source/WTF/wtf/text/StringBuilder.h 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/WTF/wtf/text/StringBuilder.h 2018-05-01 16:01:25 UTC (rev 231197)
@@ -49,9 +49,12 @@
StringBuilder& operator=(StringBuilder&&) = default;
WTF_EXPORT_PRIVATE void append(const UChar*, unsigned);
+ WTF_EXPORT_PRIVATE bool tryAppend(const UChar*, unsigned);
WTF_EXPORT_PRIVATE void append(const LChar*, unsigned);
+ WTF_EXPORT_PRIVATE bool tryAppend(const LChar*, unsigned);
ALWAYS_INLINE void append(const char* characters, unsigned length) { append(reinterpret_cast<const LChar*>(characters), length); }
+ ALWAYS_INLINE bool tryAppend(const char* characters, unsigned length) { return tryAppend(reinterpret_cast<const LChar*>(characters), length); }
void append(const AtomicString& atomicString)
{
@@ -60,8 +63,13 @@
void append(const String& string)
{
+ if (!tryAppend(string))
+ CRASH();
+ }
+ bool tryAppend(const String& string)
+ {
if (!string.length())
- return;
+ return true;
// If we're appending to an empty string, and there is not a buffer (reserveCapacity has not been called)
// then just retain the string.
@@ -69,19 +77,24 @@
m_string = string;
m_length = string.length();
m_is8Bit = m_string.is8Bit();
- return;
+ return true;
}
if (string.is8Bit())
- append(string.characters8(), string.length());
+ return tryAppend(string.characters8(), string.length());
else
- append(string.characters16(), string.length());
+ return tryAppend(string.characters16(), string.length());
}
void append(const StringBuilder& other)
{
+ if (!tryAppend(other))
+ CRASH();
+ }
+ bool tryAppend(const StringBuilder& other)
+ {
if (!other.m_length)
- return;
+ return true;
// If we're appending to an empty string, and there is not a buffer (reserveCapacity has not been called)
// then just retain the string.
@@ -88,21 +101,26 @@
if (!m_length && !m_buffer && !other.m_string.isNull()) {
m_string = other.m_string;
m_length = other.m_length;
- return;
+ return true;
}
if (other.is8Bit())
- append(other.characters8(), other.m_length);
+ return tryAppend(other.characters8(), other.m_length);
else
- append(other.characters16(), other.m_length);
+ return tryAppend(other.characters16(), other.m_length);
}
void append(StringView stringView)
{
+ if (!tryAppend(stringView))
+ CRASH();
+ }
+ bool tryAppend(StringView stringView)
+ {
if (stringView.is8Bit())
- append(stringView.characters8(), stringView.length());
+ return tryAppend(stringView.characters8(), stringView.length());
else
- append(stringView.characters16(), stringView.length());
+ return tryAppend(stringView.characters16(), stringView.length());
}
#if USE(CF)
@@ -131,6 +149,12 @@
if (characters)
append(characters, strlen(characters));
}
+ bool tryAppend(const char* characters)
+ {
+ if (characters)
+ return tryAppend(characters, strlen(characters));
+ return true;
+ }
void append(UChar c)
{
@@ -150,19 +174,23 @@
void append(LChar c)
{
+ if (!tryAppend(c))
+ CRASH();
+ }
+ bool tryAppend(LChar c)
+ {
if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
if (m_is8Bit)
m_bufferCharacters8[m_length++] = c;
else
m_bufferCharacters16[m_length++] = c;
+ return true;
} else
- append(&c, 1);
+ return tryAppend(&c, 1);
}
- void append(char c)
- {
- append(static_cast<LChar>(c));
- }
+ void append(char c) { append(static_cast<LChar>(c)); }
+ bool tryAppend(char c) { return tryAppend(static_cast<LChar>(c)); }
void append(UChar32 c)
{
@@ -174,10 +202,13 @@
append(U16_TRAIL(c));
}
- WTF_EXPORT_PRIVATE bool appendQuotedJSONString(const String&);
+ WTF_EXPORT_PRIVATE void appendQuotedJSONString(const String&);
+ WTF_EXPORT_PRIVATE bool tryAppendQuotedJSONString(const String&);
template<unsigned characterCount>
ALWAYS_INLINE void appendLiteral(const char (&characters)[characterCount]) { append(characters, characterCount - 1); }
+ template<unsigned characterCount>
+ ALWAYS_INLINE bool tryAppendLiteral(const char (&characters)[characterCount]) { return tryAppend(characters, characterCount - 1); }
WTF_EXPORT_PRIVATE void appendNumber(int);
WTF_EXPORT_PRIVATE void appendNumber(unsigned int);
@@ -298,6 +329,7 @@
void allocateBuffer(const LChar* currentCharacters, unsigned requiredLength);
void allocateBuffer(const UChar* currentCharacters, unsigned requiredLength);
void allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
+ bool tryAllocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
template <typename CharType>
void reallocateBuffer(unsigned requiredLength);
template <typename CharType>
Modified: trunk/Source/WTF/wtf/text/StringBuilderJSON.cpp (231196 => 231197)
--- trunk/Source/WTF/wtf/text/StringBuilderJSON.cpp 2018-05-01 15:47:29 UTC (rev 231196)
+++ trunk/Source/WTF/wtf/text/StringBuilderJSON.cpp 2018-05-01 16:01:25 UTC (rev 231197)
@@ -74,8 +74,14 @@
}
}
-bool StringBuilder::appendQuotedJSONString(const String& string)
+void StringBuilder::appendQuotedJSONString(const String& string)
{
+ if (!tryAppendQuotedJSONString(string))
+ CRASH();
+}
+
+bool StringBuilder::tryAppendQuotedJSONString(const String& string)
+{
// 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.
