summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDan Sinclair <dsinclair@chromium.org>2016-08-30 11:22:58 -0400
committerDan Sinclair <dsinclair@chromium.org>2016-08-30 11:22:58 -0400
commitb48a715c301758ff66a96f74e3921c121b317603 (patch)
tree91aa2e51b8b7c99e9e98620d4f2d7fe766afec69
parentfe9485fff248dd04a0643e4ce2309aec7ae237e1 (diff)
downloadpdfium-chromium/2785.tar.xz
[Merge to M53] Fixup integer conversion logic.chromium/2785
In bc8a64029f898286c3dcad3a6cecdc98ef30b139 we updated the FX_atonum logic to correctly handle integer overflow. This causes issues when parsing the Permissions flag of encrypted documents as that flag isn't encoded like other numbers. The Permissions flag is a unsigned value, and has to be treated as such since the sign bit is always set. The current logic will detect an overflow of the int value and return 0. The old logic would have detected the overflow and returned the negative result regardless. This CL updates the logic to do the string to int conversion as a uint32_t and then verifies the uint32_t value, if a sign was provided, fits within the int range, otherwise it converts it to an int and lets it be positive or negative as needed. Merge clean TBRing. BUG=pdfium:539,chromium:642256 TBR=thestig@chromium.org Review-Url: https://codereview.chromium.org/2168173002 (cherry picked from commit 6f1025492801aaa93fca2c0ed7c40a3389ad8cd1) Review URL: https://codereview.chromium.org/2291183002 .
-rw-r--r--core/fxcrt/fx_basic_util.cpp41
-rw-r--r--core/fxcrt/fx_basic_util_unittest.cpp47
2 files changed, 84 insertions, 4 deletions
diff --git a/core/fxcrt/fx_basic_util.cpp b/core/fxcrt/fx_basic_util.cpp
index abd84a864f..663ca93e9c 100644
--- a/core/fxcrt/fx_basic_util.cpp
+++ b/core/fxcrt/fx_basic_util.cpp
@@ -16,8 +16,15 @@
#include <algorithm>
#include <cctype>
+#include <limits>
#include <memory>
+namespace {
+
+const int kDefaultIntValue = 0;
+
+} // namespace
+
bool FX_atonum(const CFX_ByteStringC& strc, void* pData) {
if (strc.Find('.') != -1) {
FX_FLOAT* pFloat = static_cast<FX_FLOAT*>(pData);
@@ -25,26 +32,52 @@ bool FX_atonum(const CFX_ByteStringC& strc, void* pData) {
return false;
}
- int cc = 0;
- pdfium::base::CheckedNumeric<int> integer = 0;
+ // Note, numbers in PDF are typically of the form 123, -123, etc. But,
+ // for things like the Permissions on the encryption hash the number is
+ // actually an unsigned value. We use a uint32_t so we can deal with the
+ // unsigned and then check for overflow if the user actually signed the value.
+ // The Permissions flag is listed in Table 3.20 PDF 1.7 spec.
+ pdfium::base::CheckedNumeric<uint32_t> integer = 0;
bool bNegative = false;
+ bool bSigned = false;
+ int cc = 0;
if (strc[0] == '+') {
cc++;
+ bSigned = true;
} else if (strc[0] == '-') {
bNegative = true;
+ bSigned = true;
cc++;
}
+
while (cc < strc.GetLength() && std::isdigit(strc[cc])) {
integer = integer * 10 + FXSYS_toDecimalDigit(strc.CharAt(cc));
if (!integer.IsValid())
break;
cc++;
}
+
+ // We have a sign, and the value was greater then a regular integer
+ // we've overflowed, reset to the default value.
+ if (bSigned) {
+ if (bNegative) {
+ if (integer.ValueOrDefault(kDefaultIntValue) >
+ static_cast<uint32_t>(std::numeric_limits<int>::max()) + 1) {
+ integer = kDefaultIntValue;
+ }
+ } else if (integer.ValueOrDefault(kDefaultIntValue) >
+ static_cast<uint32_t>(std::numeric_limits<int>::max())) {
+ integer = kDefaultIntValue;
+ }
+ }
+
+ // Switch back to the int space so we can flip to a negative if we need.
+ int value = static_cast<int>(integer.ValueOrDefault(kDefaultIntValue));
if (bNegative)
- integer = -integer;
+ value = -value;
int* pInt = static_cast<int*>(pData);
- *pInt = integer.ValueOrDefault(0);
+ *pInt = value;
return true;
}
diff --git a/core/fxcrt/fx_basic_util_unittest.cpp b/core/fxcrt/fx_basic_util_unittest.cpp
index 3272eab70c..6eae6696ad 100644
--- a/core/fxcrt/fx_basic_util_unittest.cpp
+++ b/core/fxcrt/fx_basic_util_unittest.cpp
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <limits>
+
#include "core/fxcrt/include/fx_basic.h"
#include "testing/fx_string_testhelpers.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -28,3 +30,48 @@ TEST(fxge, GetBits32) {
}
}
}
+
+TEST(fxcrt, FX_atonum) {
+ int i;
+ EXPECT_TRUE(FX_atonum("10", &i));
+ EXPECT_EQ(10, i);
+
+ EXPECT_TRUE(FX_atonum("-10", &i));
+ EXPECT_EQ(-10, i);
+
+ EXPECT_TRUE(FX_atonum("+10", &i));
+ EXPECT_EQ(10, i);
+
+ EXPECT_TRUE(FX_atonum("-2147483648", &i));
+ EXPECT_EQ(std::numeric_limits<int>::min(), i);
+
+ EXPECT_TRUE(FX_atonum("2147483647", &i));
+ EXPECT_EQ(2147483647, i);
+
+ // Value overflows.
+ EXPECT_TRUE(FX_atonum("-2147483649", &i));
+ EXPECT_EQ(0, i);
+
+ // Value overflows.
+ EXPECT_TRUE(FX_atonum("+2147483648", &i));
+ EXPECT_EQ(0, i);
+
+ // Value overflows.
+ EXPECT_TRUE(FX_atonum("4223423494965252", &i));
+ EXPECT_EQ(0, i);
+
+ // No explicit sign will allow the number to go negative. This is for things
+ // like the encryption Permissions flag (Table 3.20 PDF 1.7 spec)
+ EXPECT_TRUE(FX_atonum("4294965252", &i));
+ EXPECT_EQ(-2044, i);
+
+ EXPECT_TRUE(FX_atonum("-4294965252", &i));
+ EXPECT_EQ(0, i);
+
+ EXPECT_TRUE(FX_atonum("+4294965252", &i));
+ EXPECT_EQ(0, i);
+
+ float f;
+ EXPECT_FALSE(FX_atonum("3.24", &f));
+ EXPECT_FLOAT_EQ(3.24f, f);
+}