Skip to content

Fix integer overflow and underflow bugs in arithmetic helper functions#163

Merged
Taywee merged 9 commits into
Taywee:masterfrom
metsw24-max:safe-arithmetic-security-
May 15, 2026
Merged

Fix integer overflow and underflow bugs in arithmetic helper functions#163
Taywee merged 9 commits into
Taywee:masterfrom
metsw24-max:safe-arithmetic-security-

Conversation

@metsw24-max
Copy link
Copy Markdown
Contributor

@metsw24-max metsw24-max commented May 13, 2026

This patch fixes integer overflow and underflow issues in SafeAdd and SafeMultiply, and introduces SafeSub and SafeNeg with comprehensive regression test coverage.

Changes

args.hxx

  • Reworked SafeAdd and SafeMultiply with proper signed and unsigned handling
  • Added SafeSub for subtraction overflow and underflow protection
  • Added SafeNeg for safe negation with INT_MIN overflow handling
  • Resolved MSVC warning C4018 using std::make_unsigned_t<T>

test/safe_arithmetic.cxx

  • Added regression tests for:
    • subtraction underflow
    • negation overflow
    • signed and unsigned addition edge cases
  • Expanded arithmetic validation coverage

Impact

Eliminates undefined behavior in arithmetic operations used by argument parsing and input-sensitive code paths.

@Taywee
Copy link
Copy Markdown
Owner

Taywee commented May 13, 2026

What's the point of those #elif defined(_MSC_VER) branches that are identical to the following #else sections? They look redundant to me.

@metsw24-max
Copy link
Copy Markdown
Contributor Author

I agree the #elif defined(_MSC_VER) branches were redundant, so I collapsed them into the fallback path. I also fixed the C++11 compatibility issues that showed up in Ubuntu/macOS CI by replacing std::make_unsigned_t with typename std::make_unsigned<T>::type and removing constexpr from the arithmetic helpers.

@Taywee
Copy link
Copy Markdown
Owner

Taywee commented May 15, 2026

Much better. Thanks for making those changes.

@Taywee Taywee merged commit 6483f3b into Taywee:master May 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants