Fixup integer conversion logic.

core/fxcrt/fx_basic_util.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fxcrt/include/fx_basic.h"
 #include "core/fxcrt/include/fx_ext.h"
 
 #if _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_
 #include <dirent.h>
 #include <sys/types.h>
 #else
 #include <direct.h>
 #endif
 
 #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);
     *pFloat = FX_atof(strc);
     return false;
   }
 
+  // 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 a unsigned value. We treat use uint32_t so we can deal with the

[Lei Zhang, 2016/07/21 20:39:31]

"We treat use uint32_t" does not parse English bad.

[dsinclair, 2016/07/25 13:47:59]

Done.

+  // unsigned and then check for overflow if the user actually signed the value.
+  // 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;
-  pdfium::base::CheckedNumeric<int> integer = 0;
-  bool bNegative = false;
   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 &&
+      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 = integer.ValueOrDefault(kDefaultIntValue);
   if (bNegative)
-    integer = -integer;
+    value = -value;

[Lei Zhang, 2016/07/21 20:39:31]

Do we need to worry about the value = INT_MIN case?

[dsinclair, 2016/07/25 13:47:59]

Fixed up some issues around boundary conversions. We dont' check INT_MIN, but
INT_MAX + 1 as INT_MIN is one bigger in the negative. We're comparing againts a
uint, so I have to do the comparison in the positive.

 
   int* pInt = static_cast<int*>(pData);
-  *pInt = integer.ValueOrDefault(0);
+  *pInt = value;
   return true;
 }
 
 static const FX_FLOAT fraction_scales[] = {
     0.1f,         0.01f,         0.001f,        0.0001f,
     0.00001f,     0.000001f,     0.0000001f,    0.00000001f,
     0.000000001f, 0.0000000001f, 0.00000000001f};
 int FXSYS_FractionalScaleCount() {
   return FX_ArraySize(fraction_scales);
 }
@@ skipping 244 matching lines @@
     dstShift -= 8;
     result |= *dataPtr++ << dstShift;
   }
   if (dstShift > 0) {
     bitShift = 8 - dstShift;
     bitMask = (1 << dstShift) - 1;
     result |= *dataPtr++ >> bitShift & bitMask;
   }
   return result;
 }