Skip to content

GitLab

T
tangled
Commit 957b5b30 authored by yutak's avatar yutak Committed by Commit bot
Browse files
Options

StringToNumber: Fix integer overflow on parsing INT_MIN. 

This patch adjusts the algorithm of toIntegralType(), which parses an integer
value out of a string. Previously, toIntegralType() used a positive number
as an accumulator regardless of whether we had seen the negative sign or not,
and it inverted the accumulator value if there had been a negative sign. This
causes an integer overflow when INT_MIN is specified as the source string,
because the absolute value of INT_MIN is one larger than that of INT_MAX.

This patch fixes this issue by accumulating a negative value if the result
will be negative. Also, the detection algorithm of overflows is updated so
that it can detect overflows correctly in every possible case (it used to have
a small bug that the function fails to parse INT_MIN in base 16).

Additionally, unit tests are added.

BUG=665110

Review-Url: https://codereview.chromium.org/2563643002
Cr-Commit-Position: refs/heads/master@{#437508}
parent ff8a991d
Hide whitespace changes
Inline Side-by-side
Showing with 154 additions and 19 deletions
third_party/WebKit/Source/wtf/BUILD.gn
View file @ 957b5b30
...@@ -334,6 +334,7 @@ test("wtf_unittests") { ...@@ -334,6 +334,7 @@ test("wtf_unittests") {
"text/StringBuilderTest.cpp", "text/StringBuilderTest.cpp",
"text/StringImplTest.cpp", "text/StringImplTest.cpp",
"text/StringOperatorsTest.cpp", "text/StringOperatorsTest.cpp",
"text/StringToNumberTest.cpp",
"text/StringViewTest.cpp", "text/StringViewTest.cpp",
"text/TextCodecICUTest.cpp", "text/TextCodecICUTest.cpp",
"text/TextCodecLatin1Test.cpp", "text/TextCodecLatin1Test.cpp",
......
third_party/WebKit/Source/wtf/text/StringToNumber.cpp
View file @ 957b5b30
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "wtf/ASCIICType.h" #include "wtf/ASCIICType.h"
#include "wtf/dtoa.h" #include "wtf/dtoa.h"
#include "wtf/text/StringImpl.h" #include "wtf/text/StringImpl.h"
#include <type_traits>
namespace WTF { namespace WTF {
...@@ -29,10 +30,13 @@ static inline IntegralType toIntegralType(const CharType* data, ...@@ -29,10 +30,13 @@ static inline IntegralType toIntegralType(const CharType* data,
size_t length, size_t length,
bool* ok, bool* ok,
int base) { int base) {
static const IntegralType integralMax = static_assert(std::is_integral<IntegralType>::value,
"IntegralType must be an integral type.");
static constexpr IntegralType integralMax =
std::numeric_limits<IntegralType>::max(); std::numeric_limits<IntegralType>::max();
static const bool isSigned = std::numeric_limits<IntegralType>::is_signed; static constexpr IntegralType integralMin =
const IntegralType maxMultiplier = integralMax / base; std::numeric_limits<IntegralType>::min();
static constexpr bool isSigned = std::numeric_limits<IntegralType>::is_signed;
IntegralType value = 0; IntegralType value = 0;
bool isOk = false; bool isOk = false;
...@@ -70,27 +74,31 @@ static inline IntegralType toIntegralType(const CharType* data, ...@@ -70,27 +74,31 @@ static inline IntegralType toIntegralType(const CharType* data,
else else
digitValue = c - 'A' + 10; digitValue = c - 'A' + 10;
if (value > maxMultiplier || bool overflow;
(value == maxMultiplier && if (isNegative) {
digitValue > (integralMax % base) + isNegative)) // Overflow condition:
// value * base - digitValue < integralMin
// <=> value < (integralMin + digitValue) / base
// We must be careful of rounding errors here, but the default rounding
// mode (round to zero) works well, so we can use this formula as-is.
overflow = value < (integralMin + digitValue) / base;
} else {
// Overflow condition:
// value * base + digitValue > integralMax
// <=> value > (integralMax + digitValue) / base
// Ditto regarding rounding errors.
overflow = value > (integralMax - digitValue) / base;
}
if (overflow)
goto bye; goto bye;
value = base * value + digitValue; if (isNegative)
value = base * value - digitValue;
else
value = base * value + digitValue;
++data; ++data;
} }
#if COMPILER(MSVC)
#pragma warning(push, 0)
#pragma warning(disable : 4146)
#endif
if (isNegative)
value = -value;
#if COMPILER(MSVC)
#pragma warning(pop)
#endif
// skip trailing space // skip trailing space
while (length && isSpaceOrNewline(*data)) { while (length && isSpaceOrNewline(*data)) {
--length; --length;
......
third_party/WebKit/Source/wtf/text/StringToNumberTest.cpp 0 → 100644
View file @ 957b5b30
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "wtf/text/StringToNumber.h"
#include "testing/gtest/include/gtest/gtest.h"
#include <cstring>
namespace WTF {
TEST(StringToNumberTest, TestCharactersToIntStrict) {
#define EXPECT_VALID(string, expectedValue, base) \
do { \
bool ok; \
int value = charactersToIntStrict(reinterpret_cast<const LChar*>(string), \
std::strlen(string), &ok, base); \
EXPECT_TRUE(ok); \
EXPECT_EQ(value, expectedValue); \
} while (false)
#define EXPECT_INVALID(string, base) \
do { \
bool ok; \
charactersToIntStrict(reinterpret_cast<const LChar*>(string), \
std::strlen(string), &ok, base); \
EXPECT_FALSE(ok); \
} while (false)
#define EXPECT_VALID_DECIMAL(string, expectedValue) \
EXPECT_VALID(string, expectedValue, 10)
#define EXPECT_INVALID_DECIMAL(string) EXPECT_INVALID(string, 10)
#define EXPECT_VALID_HEXADECIMAL(string, expectedValue) \
EXPECT_VALID(string, expectedValue, 16)
#define EXPECT_INVALID_HEXADECIMAL(string) EXPECT_INVALID(string, 16)
EXPECT_VALID_DECIMAL("1", 1);
EXPECT_VALID_DECIMAL("2", 2);
EXPECT_VALID_DECIMAL("9", 9);
EXPECT_VALID_DECIMAL("10", 10);
EXPECT_VALID_DECIMAL("0", 0);
EXPECT_VALID_DECIMAL("-0", 0);
EXPECT_VALID_DECIMAL("-1", -1);
EXPECT_VALID_DECIMAL("-2", -2);
EXPECT_VALID_DECIMAL("-9", -9);
EXPECT_VALID_DECIMAL("-10", -10);
EXPECT_VALID_DECIMAL("+0", 0);
EXPECT_VALID_DECIMAL("+1", 1);
EXPECT_VALID_DECIMAL("+2", 2);
EXPECT_VALID_DECIMAL("+9", 9);
EXPECT_VALID_DECIMAL("+10", 10);
EXPECT_VALID_DECIMAL("00", 0);
EXPECT_VALID_DECIMAL("+00", 0);
EXPECT_VALID_DECIMAL("-00", 0);
EXPECT_VALID_DECIMAL("01", 1);
EXPECT_VALID_DECIMAL("-01", -1);
EXPECT_VALID_DECIMAL("00000000000000000000", 0);
EXPECT_INVALID_DECIMAL("a");
EXPECT_INVALID_DECIMAL("1a");
EXPECT_INVALID_DECIMAL("a1");
EXPECT_INVALID_DECIMAL("-a");
EXPECT_INVALID_DECIMAL("");
EXPECT_INVALID_DECIMAL("-");
EXPECT_INVALID_DECIMAL("--1");
EXPECT_INVALID_DECIMAL("++1");
EXPECT_INVALID_DECIMAL("+-1");
EXPECT_INVALID_DECIMAL("-+1");
EXPECT_INVALID_DECIMAL("0-");
EXPECT_INVALID_DECIMAL("0+");
EXPECT_VALID_DECIMAL("2147483647", 2147483647);
EXPECT_VALID_DECIMAL("02147483647", 2147483647);
EXPECT_INVALID_DECIMAL("2147483648");
EXPECT_INVALID_DECIMAL("2147483649");
EXPECT_INVALID_DECIMAL("2147483650");
EXPECT_INVALID_DECIMAL("2147483700");
EXPECT_INVALID_DECIMAL("2147484000");
EXPECT_INVALID_DECIMAL("2200000000");
EXPECT_INVALID_DECIMAL("3000000000");
EXPECT_INVALID_DECIMAL("10000000000");
EXPECT_VALID_DECIMAL("-2147483647", -2147483647);
EXPECT_VALID_DECIMAL("-2147483648", -2147483647 - 1);
EXPECT_INVALID_DECIMAL("-2147483649");
EXPECT_INVALID_DECIMAL("-2147483650");
EXPECT_INVALID_DECIMAL("-2147483700");
EXPECT_INVALID_DECIMAL("-2147484000");
EXPECT_INVALID_DECIMAL("-2200000000");
EXPECT_INVALID_DECIMAL("-3000000000");
EXPECT_INVALID_DECIMAL("-10000000000");
EXPECT_VALID_HEXADECIMAL("1", 1);
EXPECT_VALID_HEXADECIMAL("a", 0xA);
EXPECT_VALID_HEXADECIMAL("A", 0xA);
EXPECT_VALID_HEXADECIMAL("+a", 0xA);
EXPECT_VALID_HEXADECIMAL("+A", 0xA);
EXPECT_VALID_HEXADECIMAL("-a", -0xA);
EXPECT_VALID_HEXADECIMAL("-A", -0xA);
EXPECT_VALID_HEXADECIMAL("7fffffff", 0x7FFFFFFF);
EXPECT_INVALID_HEXADECIMAL("80000000");
EXPECT_INVALID_HEXADECIMAL("8000000a");
EXPECT_INVALID_HEXADECIMAL("8000000f");
EXPECT_INVALID_HEXADECIMAL("90000000");
EXPECT_INVALID_HEXADECIMAL("fffffff0");
EXPECT_INVALID_HEXADECIMAL("ffffffff");
EXPECT_INVALID_HEXADECIMAL("100000000");
EXPECT_INVALID_HEXADECIMAL("7fffffff0");
EXPECT_VALID_HEXADECIMAL("-7fffffff", -0x7FFFFFFF);
EXPECT_VALID_HEXADECIMAL("-80000000", -0x7FFFFFFF - 1);
EXPECT_INVALID_HEXADECIMAL("-80000001");
EXPECT_INVALID_HEXADECIMAL("-8000000a");
EXPECT_INVALID_HEXADECIMAL("-8000000f");
EXPECT_INVALID_HEXADECIMAL("-80000010");
EXPECT_INVALID_HEXADECIMAL("-90000000");
EXPECT_INVALID_HEXADECIMAL("-f0000000");
EXPECT_INVALID_HEXADECIMAL("-fffffff0");
EXPECT_INVALID_HEXADECIMAL("-ffffffff");
#undef EXPECT_VALID_DECIMAL
#undef EXPECT_INVALID_DECIMAL
#undef EXPECT_VALID_HEXADECIMAL
#undef EXPECT_INVALID_HEXADECIMAL
#undef EXPECT_VALID
#undef EXPECT_INVALID
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment