From 583cb59c1233fb55d3ec540473ce35c6f465a5c8 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 10 Apr 2024 07:06:46 -0400 Subject: [PATCH] Move checked integer arithmetic to int_utils.h --- src/lib/asn1/ber_dec.cpp | 14 ++-- src/lib/compression/compress_utils.cpp | 6 +- src/lib/utils/allocator.cpp | 5 +- src/lib/utils/info.txt | 2 +- src/lib/utils/int_utils.h | 60 ++++++++++++++ .../locking_allocator/locking_allocator.cpp | 27 ++++--- src/lib/utils/safeint.h | 78 ------------------- 7 files changed, 88 insertions(+), 104 deletions(-) create mode 100644 src/lib/utils/int_utils.h delete mode 100644 src/lib/utils/safeint.h diff --git a/src/lib/asn1/ber_dec.cpp b/src/lib/asn1/ber_dec.cpp index f77eecf4d6c..1b92fc037d7 100644 --- a/src/lib/asn1/ber_dec.cpp +++ b/src/lib/asn1/ber_dec.cpp @@ -8,8 +8,8 @@ #include #include +#include #include -#include #include namespace Botan { @@ -128,18 +128,20 @@ size_t find_eoc(DataSource* ber, size_t allow_indef) { while(true) { ASN1_Type type_tag; ASN1_Class class_tag; - size_t tag_size = decode_tag(&source, type_tag, class_tag); + const size_t tag_size = decode_tag(&source, type_tag, class_tag); if(type_tag == ASN1_Type::NoObject) { break; } size_t length_size = 0; - size_t item_size = decode_length(&source, length_size, allow_indef); + const size_t item_size = decode_length(&source, length_size, allow_indef); source.discard_next(item_size); - length = BOTAN_CHECKED_ADD(length, item_size); - length = BOTAN_CHECKED_ADD(length, tag_size); - length = BOTAN_CHECKED_ADD(length, length_size); + if(auto new_len = checked_add(length, item_size, tag_size, length_size)) { + length = new_len.value(); + } else { + throw Decoding_Error("Integer overflow while decoding DER"); + } if(type_tag == ASN1_Type::Eoc && class_tag == ASN1_Class::Universal) { break; diff --git a/src/lib/compression/compress_utils.cpp b/src/lib/compression/compress_utils.cpp index eac5add84de..cc89beb8c0e 100644 --- a/src/lib/compression/compress_utils.cpp +++ b/src/lib/compression/compress_utils.cpp @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include namespace Botan { @@ -19,7 +19,9 @@ Compression_Error::Compression_Error(const char* func_name, ErrorType type, int Exception(fmt("Compression API {} failed with return code {}", func_name, rc)), m_type(type), m_rc(rc) {} void* Compression_Alloc_Info::do_malloc(size_t n, size_t size) { - if(!BOTAN_CHECKED_MUL(n, size).has_value()) [[unlikely]] { + // Precheck for integer overflow in the multiplication + // before passing to calloc, which may or may not check. + if(!checked_mul(n, size)) { return nullptr; } diff --git a/src/lib/utils/allocator.cpp b/src/lib/utils/allocator.cpp index 512e830a357..8f624896b6f 100644 --- a/src/lib/utils/allocator.cpp +++ b/src/lib/utils/allocator.cpp @@ -7,7 +7,7 @@ #include #include -#include +#include #include #include @@ -23,8 +23,7 @@ BOTAN_MALLOC_FN void* allocate_memory(size_t elems, size_t elem_size) { } // Some calloc implementations do not check for overflow (?!?) - - if(!BOTAN_CHECKED_MUL(elems, elem_size).has_value()) { + if(!checked_mul(elems, elem_size).has_value()) { throw std::bad_alloc(); } diff --git a/src/lib/utils/info.txt b/src/lib/utils/info.txt index 11374c0c8f4..4c4fcc733f5 100644 --- a/src/lib/utils/info.txt +++ b/src/lib/utils/info.txt @@ -35,6 +35,7 @@ ct_utils.h donna128.h filesystem.h fmt.h +int_utils.h loadstor.h mul128.h os_utils.h @@ -42,7 +43,6 @@ parsing.h prefetch.h rotate.h rounding.h -safeint.h scan_name.h stl_util.h timer.h diff --git a/src/lib/utils/int_utils.h b/src/lib/utils/int_utils.h new file mode 100644 index 00000000000..95b31e16db5 --- /dev/null +++ b/src/lib/utils/int_utils.h @@ -0,0 +1,60 @@ +/* +* (C) 2024 Jack Lloyd +* +* Botan is released under the Simplified BSD License (see license.txt) +*/ + +#ifndef BOTAN_INT_UTILS_H_ +#define BOTAN_INT_UTILS_H_ + +#include +#include +#include + +namespace Botan { + +template +constexpr inline std::optional checked_add(T a, T b) { + const T r = a + b; + if(r - a != b) { + return {}; + } + return r; +} + +template + requires all_same_v +constexpr inline std::optional checked_add(T a, T b, Ts... rest) { + if(auto r = checked_add(a, b)) { + return checked_add(r.value(), rest...); + } else { + return {}; + } +} + +template +constexpr inline std::optional checked_mul(T a, T b) { + // Multiplication by 1U is a hack to work around C's insane + // integer promotion rules. + // https://stackoverflow.com/questions/24795651 + const T r = (1U * a) * b; + // If a == 0 then the multiply certainly did not overflow + // Otherwise r / a == b unless overflow occured + if(a != 0 && r / a != b) { + return {}; + } + return r; +} + +template +std::optional checked_cast(T input) { + const RT cast = static_cast(input); + if(static_cast(cast) != input) { + return {}; + } + return cast; +} + +} // namespace Botan + +#endif diff --git a/src/lib/utils/locking_allocator/locking_allocator.cpp b/src/lib/utils/locking_allocator/locking_allocator.cpp index 93342f079c7..ad95547fd4f 100644 --- a/src/lib/utils/locking_allocator/locking_allocator.cpp +++ b/src/lib/utils/locking_allocator/locking_allocator.cpp @@ -7,9 +7,9 @@ #include +#include #include #include -#include namespace Botan { @@ -18,12 +18,12 @@ void* mlock_allocator::allocate(size_t num_elems, size_t elem_size) { return nullptr; } - const auto n = BOTAN_CHECKED_MUL(num_elems, elem_size); - if(!n.has_value()) { - return nullptr; // overflow! + if(auto n = checked_mul(num_elems, elem_size)) { + return m_pool->allocate(n.value()); + } else { + // overflow! + return nullptr; } - - return m_pool->allocate(n.value()); } bool mlock_allocator::deallocate(void* p, size_t num_elems, size_t elem_size) noexcept { @@ -31,16 +31,15 @@ bool mlock_allocator::deallocate(void* p, size_t num_elems, size_t elem_size) no return false; } - /* - We return nullptr in allocate if there was an overflow, so if an - overflow occurs here we know the pointer was not allocated by this pool. - */ - const auto n = BOTAN_CHECKED_MUL(num_elems, elem_size); - if(!n.has_value()) { + if(auto n = checked_mul(num_elems, elem_size)) { + return m_pool->deallocate(p, n.value()); + } else { + /* + We return nullptr in allocate if there was an overflow, so if an + overflow occurs here we know the pointer was not allocated by this pool. + */ return false; } - - return m_pool->deallocate(p, n.value()); } mlock_allocator::mlock_allocator() { diff --git a/src/lib/utils/safeint.h b/src/lib/utils/safeint.h deleted file mode 100644 index 3fa7547cc29..00000000000 --- a/src/lib/utils/safeint.h +++ /dev/null @@ -1,78 +0,0 @@ -/* -* Safe(r) Integer Handling -* (C) 2016 Jack Lloyd -* -* Botan is released under the Simplified BSD License (see license.txt) -*/ - -#ifndef BOTAN_UTILS_SAFE_INT_H_ -#define BOTAN_UTILS_SAFE_INT_H_ - -#include -#include -#include -#include - -#if defined(_MSC_VER) - #include -#endif - -namespace Botan { - -class Integer_Overflow_Detected final : public Exception { - public: - Integer_Overflow_Detected(std::string_view file, int line) : - Exception(fmt("Integer overflow detected at {}:{}", file, line)) {} - - ErrorType error_type() const noexcept override { return ErrorType::InternalError; } -}; - -inline size_t checked_add(size_t x, size_t y, const char* file, int line) { -#if BOTAN_COMPILER_HAS_BUILTIN(__builtin_add_overflow) - size_t z; - if(__builtin_add_overflow(x, y, &z)) [[unlikely]] -#elif defined(_MSC_VER) - size_t z; - if(SizeTAdd(x, y, &z) != S_OK) [[unlikely]] -#else - size_t z = x + y; - if(z < x) [[unlikely]] -#endif - { - throw Integer_Overflow_Detected(file, line); - } - return z; -} - -inline std::optional checked_mul(size_t x, size_t y) { -#if BOTAN_COMPILER_HAS_BUILTIN(__builtin_add_overflow) - size_t z; - if(__builtin_mul_overflow(x, y, &z)) [[unlikely]] -#elif defined(_MSC_VER) - size_t z; - if(SizeTMult(x, y, &z) != S_OK) [[unlikely]] -#else - size_t z = x * y; - if(y && z / y != x) [[unlikely]] -#endif - { - return std::nullopt; - } - return z; -} - -template -RT checked_cast_to(AT i) { - RT c = static_cast(i); - if(i != static_cast(c)) { - throw Internal_Error("Error during integer conversion"); - } - return c; -} - -#define BOTAN_CHECKED_ADD(x, y) checked_add(x, y, __FILE__, __LINE__) -#define BOTAN_CHECKED_MUL(x, y) checked_mul(x, y) - -} // namespace Botan - -#endif