diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index c97642b9930..74a38b79c67 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -631,9 +631,9 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons if (!var || var->isPointer() || (astIsContainer(bufTok) && var->getTypeName() != "std::array")) return ValueFlow::Value(-1); - const MathLib::bigint dim = std::accumulate(var->dimensions().cbegin(), var->dimensions().cend(), MathLib::bigint(1), [](MathLib::bigint i1, const Dimension &dim) { - return i1 * dim.num; - }); + MathLib::bigint dim = 0; + if (!var->dimensionsProduct(dim)) + return ValueFlow::Value(-1); ValueFlow::Value v; v.setKnown(); diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 0043f5a3cde..2b64c5f7409 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -43,7 +43,6 @@ #include #include #include -#include class Settings; @@ -1638,9 +1637,11 @@ static void setValues(const Tokenizer &tokenizer, const SymbolDatabase *symbolDa MathLib::bigint typeSize = 0; for (const Variable &var: scope.varlist) { - const MathLib::bigint mul = std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), MathLib::bigint(1), [](MathLib::bigint v, const Dimension& dim) { - return v * dim.num; - }); + MathLib::bigint mul = 0; + if (!var.dimensionsProduct(mul)) { + typeSize = 0; + break; + } if (var.valueType()) typeSize += mul * var.valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer); } diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 26801640e99..02787d8d238 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -2515,6 +2516,22 @@ bool Variable::isMember() const { return mScope && mScope->isClassOrStructOrUnion(); } +bool Variable::dimensionsProduct(MathLib::bigint &product) const +{ + const MathLib::bigint maxValue = std::numeric_limits::max(); + const MathLib::bigint minValue = std::numeric_limits::min(); + product = 1; + for (const Dimension &dim : mDimensions) { + const MathLib::bigint n = dim.num; + if (product > 0 && (n > 0 ? product > maxValue / n : n < minValue / product)) + return false; + if (product < 0 && (n > 0 ? product < minValue / n : (n < 0 && product < maxValue / n))) + return false; + product *= n; + } + return true; +} + bool Variable::isPointerArray() const { if (!isArray()) @@ -8698,9 +8715,9 @@ static Result accumulateStructMembers(const Scope* scope, F f, ValueType::Accura if (const ValueType* vt = var.valueType()) { if (vt->type == ValueType::Type::RECORD && vt->typeScope == scope) return {0, false}; - const MathLib::bigint dim = std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), MathLib::bigint(1), [](MathLib::bigint i1, const Dimension& dim) { - return i1 * dim.num; - }); + MathLib::bigint dim = 0; + if (!var.dimensionsProduct(dim)) + return {0, false}; if (var.nameToken()->scope() != scope && var.nameToken()->scope()->definedType) { // anonymous union const auto ret = anonScopes.insert(var.nameToken()->scope()); if (ret.second) diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 832bcddeab3..c4199b223fa 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -547,6 +547,13 @@ class CPPCHECKLIB Variable { return mDimensions; } + /** + * Get the number of array elements, the product of all dimension lengths. + * @param[out] product the number of elements + * @return false if the product does not fit in a MathLib::bigint + */ + bool dimensionsProduct(MathLib::bigint &product) const; + /** * Get array dimension length. * @return length of dimension diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 0e0bf5ce385..44c67eff08b 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -271,6 +271,7 @@ class TestBufferOverrun : public TestFixture { TEST_CASE(minsize_strlen); TEST_CASE(minsize_mul); TEST_CASE(minsize_overflow); + TEST_CASE(arraysize_overflow); TEST_CASE(unknownType); TEST_CASE(terminateStrncpy1); @@ -4990,6 +4991,21 @@ class TestBufferOverrun : public TestFixture { ASSERT_EQUALS("[test.cpp:3:19]: (error) Buffer is accessed out of bounds: buf [bufferAccessOutOfBounds]\n", errout_str()); } + void arraysize_overflow() { + // the number of array elements must not wrap around + check("void f() {\n" + " char a[0x4000000000000001][4];\n" + " memset(a, 0, 10);\n" + "}\n", settings0); + ASSERT_EQUALS("", errout_str()); + + check("void f() {\n" + " char a[0x100000000][0x100000000];\n" + " memset(a, 0, 10);\n" + "}\n", settings0); + ASSERT_EQUALS("", errout_str()); + } + // extracttests.enable void unknownType() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 1e953af1535..0aa9bc1aa70 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1864,6 +1864,13 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(1U, values.size()); ASSERT_EQUALS(settings.platform.sizeof_int, values.back().intvalue); ASSERT_EQUALS_ENUM(ValueFlow::Value::ValueKind::Known, values.back().valueKind); + + // the number of array elements does not fit in a MathLib::bigint + code = "struct S { char m[0x4000000000000001][4]; };\n" + "x = sizeof(S);\n"; + values = tokenValues(code, "( S"); + ASSERT_EQUALS(1U, values.size()); + ASSERT(values.back().isImpossible()); } void valueFlowComma()