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 <caol...@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 <c...@libreoffice.org> Reviewed-by: Caolán McNamara <caol...@redhat.com> Tested-by: Caolán McNamara <caol...@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; }
_______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits