From 1086fac0cd0858c28ac7a4644b5dea8ca1bd82a9 Mon Sep 17 00:00:00 2001 From: nussaiba shaikh Date: Tue, 13 Oct 2026 15:54:21 +0530 Subject: [PATCH] check signed overflow in MSBuild arithmetic intrinsics --- lib/importproject.cpp | 73 ++++++++++++++++++++++++++++++++++---- test/testimportproject.cpp | 8 +++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/lib/importproject.cpp b/lib/importproject.cpp index d81c95ab6fd..af18e34a243 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -848,6 +849,44 @@ static std::string safeFormat(const char *fmt, Args... args) { #pragma clang diagnostic pop #endif +// Checked signed arithmetic for the [MSBuild]::Add/Subtract/Multiply intrinsics. +// Operands come straight from the (untrusted) project file, so the result can +// exceed long long and the plain a+b / a-b / a*b would be signed-overflow UB. +// These return false on overflow so the caller can report it instead. +static bool addChecked(long long a, long long b, long long &res) { + if ((b > 0 && a > LLONG_MAX - b) || (b < 0 && a < LLONG_MIN - b)) + return false; + res = a + b; + return true; +} + +static bool subChecked(long long a, long long b, long long &res) { + if ((b < 0 && a > LLONG_MAX + b) || (b > 0 && a < LLONG_MIN + b)) + return false; + res = a - b; + return true; +} + +static bool mulChecked(long long a, long long b, long long &res) { + if (a > 0) { + if (b > 0) { + if (a > LLONG_MAX / b) + return false; + } else if (b < LLONG_MIN / a) { + return false; + } + } else { + if (b > 0) { + if (a < LLONG_MIN / b) + return false; + } else if (a != 0 && b < LLONG_MAX / a) { + return false; + } + } + res = a * b; + return true; +} + // Evaluate a $([ClassName]::Method(args)) static property function. // Returns an empty string for unknown or unimplementable functions rather // than throwing, so import can continue gracefully. @@ -893,17 +932,37 @@ std::string ImportProject::applyMSBuildStaticFunction(const std::string &classNa if (args.size() == 2) { long long a = 0, b = 0; if (toInt(args[0], a) && toInt(args[1], b)) { - if (caseInsensitiveStringCompare(member, "Add") == 0) - return std::to_string(a + b); - if (caseInsensitiveStringCompare(member, "Subtract") == 0) - return std::to_string(a - b); - if (caseInsensitiveStringCompare(member, "Multiply") == 0) - return std::to_string(a * b); + long long r = 0; + if (caseInsensitiveStringCompare(member, "Add") == 0) { + if (!addChecked(a, b, r)) { + addDebug("MSBuild::Add: integer overflow"); + return std::string(); + } + return std::to_string(r); + } + if (caseInsensitiveStringCompare(member, "Subtract") == 0) { + if (!subChecked(a, b, r)) { + addDebug("MSBuild::Subtract: integer overflow"); + return std::string(); + } + return std::to_string(r); + } + if (caseInsensitiveStringCompare(member, "Multiply") == 0) { + if (!mulChecked(a, b, r)) { + addDebug("MSBuild::Multiply: integer overflow"); + return std::string(); + } + return std::to_string(r); + } if (caseInsensitiveStringCompare(member, "Divide") == 0) { if (b == 0) { addDebug("MSBuild::Divide: division by zero"); return std::string(); } + if (a == LLONG_MIN && b == -1) { + addDebug("MSBuild::Divide: integer overflow"); + return std::string(); + } return std::to_string(a / b); } if (caseInsensitiveStringCompare(member, "Modulo") == 0) { @@ -911,6 +970,8 @@ std::string ImportProject::applyMSBuildStaticFunction(const std::string &classNa addDebug("MSBuild::Modulo: division by zero"); return std::string(); } + if (a == LLONG_MIN && b == -1) + return std::string("0"); return std::to_string(a % b); } } diff --git a/test/testimportproject.cpp b/test/testimportproject.cpp index a9b756ff17a..9dad49099de 100644 --- a/test/testimportproject.cpp +++ b/test/testimportproject.cpp @@ -1211,6 +1211,14 @@ class TestImportProject : public TestFixture { ASSERT_EQUALS("3", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Divide(9, 3))")); ASSERT_EQUALS("2", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(5, 3))")); + // Operands that would overflow long long must not compute a signed-overflow + // (undefined-behavior) result; the operation yields the empty string instead. + ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Add(9223372036854775807, 1))")); + ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Subtract(-9223372036854775808, 1))")); + ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Multiply(9999999999, 9999999999))")); + ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Divide(-9223372036854775808, -1))")); + ASSERT_EQUALS("0", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(-9223372036854775808, -1))")); + // --- $([MSBuild]::...) path helpers --- ASSERT_EQUALS("foo/", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::EnsureTrailingSlash('foo'))")); ASSERT_EQUALS("foo/", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::EnsureTrailingSlash('foo/'))"));