From 1086fac0cd0858c28ac7a4644b5dea8ca1bd82a9 Mon Sep 17 00:00:00 2001 From: nussaiba shaikh Date: Tue, 13 Oct 2026 15:54:21 +0530 Subject: [PATCH 1/2] 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/'))")); From cd027913310b05995b78e304e1eae11b9b1c66aa Mon Sep 17 00:00:00 2001 From: nussaiba shaikh Date: Tue, 20 Oct 2026 22:26:44 +0530 Subject: [PATCH 2/2] wrap on overflow in MSBuild Add/Subtract/Multiply to match MSBuild --- lib/importproject.cpp | 76 ++++++++++---------------------------- test/testimportproject.cpp | 12 +++--- 2 files changed, 26 insertions(+), 62 deletions(-) diff --git a/lib/importproject.cpp b/lib/importproject.cpp index af18e34a243..24597834694 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -849,42 +849,20 @@ 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; +// MSBuild evaluates the [MSBuild]::Add/Subtract/Multiply intrinsics in .NET's +// default unchecked context, so on overflow the result wraps (two's complement). +// Do the arithmetic on unsigned operands to get the same result without +// signed-overflow UB. +static long long wrapAdd(long long a, long long b) { + return static_cast(static_cast(a) + static_cast(b)); } -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 long long wrapSub(long long a, long long b) { + return static_cast(static_cast(a) - static_cast(b)); } -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; +static long long wrapMul(long long a, long long b) { + return static_cast(static_cast(a) * static_cast(b)); } // Evaluate a $([ClassName]::Method(args)) static property function. @@ -932,28 +910,12 @@ 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)) { - 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, "Add") == 0) + return std::to_string(wrapAdd(a, b)); + if (caseInsensitiveStringCompare(member, "Subtract") == 0) + return std::to_string(wrapSub(a, b)); + if (caseInsensitiveStringCompare(member, "Multiply") == 0) + return std::to_string(wrapMul(a, b)); if (caseInsensitiveStringCompare(member, "Divide") == 0) { if (b == 0) { addDebug("MSBuild::Divide: division by zero"); @@ -970,8 +932,10 @@ 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"); + if (a == LLONG_MIN && b == -1) { + addDebug("MSBuild::Modulo: integer overflow"); + return std::string(); + } return std::to_string(a % b); } } diff --git a/test/testimportproject.cpp b/test/testimportproject.cpp index 9dad49099de..2ecdf677381 100644 --- a/test/testimportproject.cpp +++ b/test/testimportproject.cpp @@ -1211,13 +1211,13 @@ 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))")); + // Overflow wraps around like MSBuild (.NET unchecked arithmetic). + ASSERT_EQUALS("-9223372036854775808", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Add(9223372036854775807, 1))")); + ASSERT_EQUALS("9223372036854775807", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Subtract(-9223372036854775808, 1))")); + ASSERT_EQUALS("7766279611452241921", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Multiply(9999999999, 9999999999))")); + // .NET throws OverflowException for LLONG_MIN / -1 and LLONG_MIN % -1. ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Divide(-9223372036854775808, -1))")); - ASSERT_EQUALS("0", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(-9223372036854775808, -1))")); + ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(-9223372036854775808, -1))")); // --- $([MSBuild]::...) path helpers --- ASSERT_EQUALS("foo/", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::EnsureTrailingSlash('foo'))"));