[Libreoffice-commits] core.git: include/o3tl tools/source

[Caolán McNamara]

 include/o3tl/safeint.hxx       |   89 +++++++++++++++++++++++++++++++++++++++++
 tools/source/generic/fract.cxx |   14 ++++--
 2 files changed, 98 insertions(+), 5 deletions(-)

New commits:
commit 4f0b226600fdad4e5aef9313fe8754c765cfee42
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Tue Mar 21 11:23:09 2017 +0000

    check for overflow in multiply
    
    gcc >= 5 and clang have builtins for this
    msvc has safeint.h and functions in the msl::utilties namespace
    otherwise fall back to certs demo implementations
    
    Change-Id: I6001a278c24b0be4b381d933d256f01f91ead55d
    Reviewed-on: https://gerrit.libreoffice.org/35505
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/include/o3tl/safeint.hxx b/include/o3tl/safeint.hxx
new file mode 100644
index 000000000000..552c834bd6a9
--- /dev/null
+++ b/include/o3tl/safeint.hxx
@@ -0,0 +1,89 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#ifndef INCLUDED_O3TL_SAFE_INT_HXX
+#define INCLUDED_O3TL_SAFE_INT_HXX
+
+#include <limits>
+#if defined(_MSC_VER)
+#include <safeint.h>
+#else
+#ifndef __has_builtin
+#   define __has_builtin(x) 0
+#endif
+#endif
+
+namespace o3tl
+{
+
+#if defined(_MSC_VER)
+
+template<typename T> inline bool checked_multiply(T a, T b, T& result)
+{
+    return !msl::utilities::SafeMultiply(a, b, result);
+}
+
+#elif (defined __GNUC__ && __GNUC__ >= 5) || (__has_builtin(__builtin_mul_overflow))
+
+template<typename T> inline bool checked_multiply(T a, T b, T& result)
+{
+    return __builtin_mul_overflow(a, b, &result);
+}
+
+#else
+
+//https://www.securecoding.cert.org/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow
+template<typename T> inline typename std::enable_if<std::is_signed<T>::value, bool>::type checked_multiply(T a, T b, T& result)
+{
+  if (a > 0) {  /* a is positive */
+    if (b > 0) {  /* a and b are positive */
+      if (a > (std::numeric_limits<T>::max() / b)) {
+        return true; /* Handle error */
+      }
+    } else { /* a positive, b nonpositive */
+      if (b < (std::numeric_limits<T>::min() / a)) {
+        return true; /* Handle error */
+      }
+    } /* a positive, b nonpositive */
+  } else { /* a is nonpositive */
+    if (b > 0) { /* a is nonpositive, b is positive */
+      if (a < (std::numeric_limits<T>::min() / b)) {
+        return true; /* Handle error */
+      }
+    } else { /* a and b are nonpositive */
+      if ( (a != 0) && (b < (std::numeric_limits<T>::max() / a))) {
+        return true; /* Handle error */
+      }
+    } /* End if a and b are nonpositive */
+  } /* End if a is nonpositive */
+
+  result = a * b;
+
+  return false;
+}
+
+//https://www.securecoding.cert.org/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap
+template<typename T> inline typename std::enable_if<std::is_unsigned<T>::value, bool>::type checked_multiply(T a, T b, T& result)
+{
+    if (b && a > std::numeric_limits<T>::max() / b) {
+        return true;/* Handle error */
+    }
+
+    result = a * b;
+
+    return false;
+}
+
+#endif
+
+}
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/tools/source/generic/fract.cxx b/tools/source/generic/fract.cxx
index 068a2b6429a8..bc9bef467e86 100644
--- a/tools/source/generic/fract.cxx
+++ b/tools/source/generic/fract.cxx
@@ -21,6 +21,7 @@
 #include <tools/debug.hxx>
 #include <tools/lineend.hxx>
 #include <tools/stream.hxx>
+#include <o3tl/safeint.hxx>
 #include <rtl/ustring.hxx>
 #include <sal/log.hxx>
 #include <osl/diagnose.h>
@@ -172,7 +173,7 @@ Fraction& Fraction::operator -= ( const Fraction& rVal )
 
 namespace
 {
-    template<typename T> void multiply_by(boost::rational<T>& i, const boost::rational<T>& r)
+    template<typename T> bool checked_multiply_by(boost::rational<T>& i, const boost::rational<T>& r)
     {
         // Protect against self-modification
         T num = r.numerator();
@@ -181,10 +182,13 @@ namespace
         // Avoid overflow and preserve normalization
         T gcd1 = boost::integer::gcd(i.numerator(), den);
         T gcd2 = boost::integer::gcd(num, i.denominator());
-        num = (i.numerator() / gcd1) * (num / gcd2);
-        den = (i.denominator() / gcd2) * (den / gcd1);
 
+        bool fail = false;
+        fail |= o3tl::checked_multiply(i.numerator() / gcd1, num / gcd2, num);
+        fail |= o3tl::checked_multiply(i.denominator() / gcd2, den / gcd1, den);
         i.assign(num, den);
+
+        return fail;
     }
 }
 
@@ -199,9 +203,9 @@ Fraction& Fraction::operator *= ( const Fraction& rVal )
         return *this;
     }
 
-    multiply_by(mpImpl->value, rVal.mpImpl->value);
+    bool bFail = checked_multiply_by(mpImpl->value, rVal.mpImpl->value);
 
-    if (HasOverflowValue())
+    if (bFail || HasOverflowValue())
     {
         mpImpl->valid = false;
     }