From ec24b4e336737068bfc0d234c8cf8efdd71f1722 Mon Sep 17 00:00:00 2001
From: ololuki <ololuki@gmail.com>
Date: Wed, 17 Jan 2024 08:36:32 +0100
Subject: [PATCH] Fix overflow error in integer_parser.

Checking for overflow should be done before integer overflows.
There are two checks:
(result > limit / base) is used for limits greater than rounded up to base,
e.g. for 65535 it will activate for 65540 and higher.
(result * base > limit - digit) is used for limit+1 to limit+n below
next base rounded number, e.g. 65536 up to 65539.
---
 include/cxxopts.hpp | 20 +++++++++++++++++---
 test/options.cpp    | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/cxxopts.hpp b/include/cxxopts.hpp
index 481826c..2418527 100644
--- a/include/cxxopts.hpp
+++ b/include/cxxopts.hpp
@@ -27,6 +27,7 @@ THE SOFTWARE.
 #ifndef CXXOPTS_HPP_INCLUDED
 #define CXXOPTS_HPP_INCLUDED
 
+#include <cstdlib>
 #include <cstring>
 #include <exception>
 #include <limits>
@@ -972,13 +973,26 @@ integer_parser(const std::string& text, T& value)
       throw_or_mimic<exceptions::incorrect_argument_type>(text);
     }
 
-    const US next = static_cast<US>(result * base + digit);
-    if (result > next)
+    US limit = 0;
+    if (negative)
+    {
+      limit = static_cast<US>(std::abs(static_cast<intmax_t>(std::numeric_limits<T>::min())));
+    }
+    else
+    {
+      limit = std::numeric_limits<T>::max();
+    }
+
+    if (base != 0 && result > limit / base)
+    {
+      throw_or_mimic<exceptions::incorrect_argument_type>(text);
+    }
+    if (result * base > limit - digit)
     {
       throw_or_mimic<exceptions::incorrect_argument_type>(text);
     }
 
-    result = next;
+    result = static_cast<US>(result * base + digit);
   }
 
   detail::check_signed_range<T>(negative, result, text);
diff --git a/test/options.cpp b/test/options.cpp
index 26faaf7..caad3dd 100644
--- a/test/options.cpp
+++ b/test/options.cpp
@@ -552,7 +552,11 @@ TEST_CASE("Overflow on boundary", "[integer]")
   using namespace cxxopts::values;
 
   int8_t si;
+  int16_t si16;
+  int64_t si64;
   uint8_t ui;
+  uint16_t ui16;
+  uint64_t ui64;
 
   CHECK_THROWS_AS((integer_parser("128", si)), cxxopts::exceptions::incorrect_argument_type);
   CHECK_THROWS_AS((integer_parser("-129", si)), cxxopts::exceptions::incorrect_argument_type);
@@ -560,6 +564,20 @@ TEST_CASE("Overflow on boundary", "[integer]")
   CHECK_THROWS_AS((integer_parser("-0x81", si)), cxxopts::exceptions::incorrect_argument_type);
   CHECK_THROWS_AS((integer_parser("0x80", si)), cxxopts::exceptions::incorrect_argument_type);
   CHECK_THROWS_AS((integer_parser("0x100", ui)), cxxopts::exceptions::incorrect_argument_type);
+
+  CHECK_THROWS_AS((integer_parser("65536", ui16)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("75536", ui16)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("32768", si16)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("-32769", si16)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("-42769", si16)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("-75536", si16)), cxxopts::exceptions::incorrect_argument_type);
+
+  CHECK_THROWS_AS((integer_parser("18446744073709551616", ui64)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("28446744073709551616", ui64)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("9223372036854775808", si64)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("-9223372036854775809", si64)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("-10223372036854775809", si64)), cxxopts::exceptions::incorrect_argument_type);
+  CHECK_THROWS_AS((integer_parser("-28446744073709551616", si64)), cxxopts::exceptions::incorrect_argument_type);
 }
 
 TEST_CASE("Integer overflow", "[options]")