From 84eab8bed1ff044aa3c6183b3b51355635b9d652 Mon Sep 17 00:00:00 2001 From: chrchr Date: Wed, 20 Mar 2024 15:32:10 +0100 Subject: [PATCH 1/3] Fix #12534 fuzzing crash (stack overflow) in isConstStatement() --- lib/checkother.cpp | 12 ++++++++---- .../crash-0f0efd6e66bd115fc0aabbfe6503230b31de5f24 | 1 + test/testincompletestatement.cpp | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 test/cli/fuzz-crash/crash-0f0efd6e66bd115fc0aabbfe6503230b31de5f24 diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 331cb03be27..b0535c4e14c 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1824,7 +1824,7 @@ static bool isConstant(const Token* tok) { return tok && (tok->isEnumerator() || Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL")); } -static bool isConstStatement(const Token *tok) +static bool isConstStatement(const Token *tok, bool checkParent = true) { if (!tok) return false; @@ -1875,9 +1875,13 @@ static bool isConstStatement(const Token *tok) if (Token::simpleMatch(tok, "?") && Token::simpleMatch(tok->astOperand2(), ":")) // ternary operator return isConstStatement(tok->astOperand1()) && isConstStatement(tok->astOperand2()->astOperand1()) && isConstStatement(tok->astOperand2()->astOperand2()); if (isBracketAccess(tok) && isWithoutSideEffects(tok->astOperand1(), /*checkArrayAccess*/ true, /*checkReference*/ false)) { - if (Token::simpleMatch(tok->astParent(), "[")) - return isConstStatement(tok->astOperand2()) && isConstStatement(tok->astParent()); - return isConstStatement(tok->astOperand2()); + const bool isChained = succeeds(tok->astParent(), tok); + if (Token::simpleMatch(tok->astParent(), "[")) { + if (isChained) + return isConstStatement(tok->astOperand2()) && isConstStatement(tok->astParent()); + return checkParent && isConstStatement(tok->astOperand2()); + } + return isConstStatement(tok->astOperand2(), /*checkParent*/ isChained); } return false; } diff --git a/test/cli/fuzz-crash/crash-0f0efd6e66bd115fc0aabbfe6503230b31de5f24 b/test/cli/fuzz-crash/crash-0f0efd6e66bd115fc0aabbfe6503230b31de5f24 new file mode 100644 index 00000000000..a011f109bd7 --- /dev/null +++ b/test/cli/fuzz-crash/crash-0f0efd6e66bd115fc0aabbfe6503230b31de5f24 @@ -0,0 +1 @@ +d f(t*a){a[a[3]]} \ No newline at end of file diff --git a/test/testincompletestatement.cpp b/test/testincompletestatement.cpp index a19cf6088df..df156f61d9b 100644 --- a/test/testincompletestatement.cpp +++ b/test/testincompletestatement.cpp @@ -712,6 +712,12 @@ class TestIncompleteStatement : public TestFixture { "}\n"); ASSERT_EQUALS("[test.cpp:3]: (warning) Redundant code: Found a statement that begins with enumerator constant.\n", errout_str()); + + check("void f(int* a) {\n" // #12534 + " a[a[3]];\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Redundant code: Found unused array access.\n", + errout_str()); } void vardecl() { From 8f1db84ee0c2f9a3625a8749fb75c8675ed87d40 Mon Sep 17 00:00:00 2001 From: chrchr Date: Wed, 20 Mar 2024 16:42:41 +0100 Subject: [PATCH 2/3] Simplify --- lib/checkother.cpp | 6 +++--- test/testincompletestatement.cpp | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index b0535c4e14c..fc087585f28 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1824,7 +1824,7 @@ static bool isConstant(const Token* tok) { return tok && (tok->isEnumerator() || Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL")); } -static bool isConstStatement(const Token *tok, bool checkParent = true) +static bool isConstStatement(const Token *tok, bool isNestedBracket = false) { if (!tok) return false; @@ -1879,9 +1879,9 @@ static bool isConstStatement(const Token *tok, bool checkParent = true) if (Token::simpleMatch(tok->astParent(), "[")) { if (isChained) return isConstStatement(tok->astOperand2()) && isConstStatement(tok->astParent()); - return checkParent && isConstStatement(tok->astOperand2()); + return !isNestedBracket && isConstStatement(tok->astOperand2()); } - return isConstStatement(tok->astOperand2(), /*checkParent*/ isChained); + return isConstStatement(tok->astOperand2(), /*isNestedBracket*/ !isChained); } return false; } diff --git a/test/testincompletestatement.cpp b/test/testincompletestatement.cpp index df156f61d9b..f30e024e113 100644 --- a/test/testincompletestatement.cpp +++ b/test/testincompletestatement.cpp @@ -715,6 +715,7 @@ class TestIncompleteStatement : public TestFixture { check("void f(int* a) {\n" // #12534 " a[a[3]];\n" + " a[a[g()]];\n" "}\n"); ASSERT_EQUALS("[test.cpp:2]: (warning) Redundant code: Found unused array access.\n", errout_str()); From 175ec11f612fc4dae4f985f1055c6cd67b41cb32 Mon Sep 17 00:00:00 2001 From: chrchr Date: Wed, 20 Mar 2024 16:47:17 +0100 Subject: [PATCH 3/3] Fix location --- lib/checkother.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index fc087585f28..1995519e66e 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1879,7 +1879,7 @@ static bool isConstStatement(const Token *tok, bool isNestedBracket = false) if (Token::simpleMatch(tok->astParent(), "[")) { if (isChained) return isConstStatement(tok->astOperand2()) && isConstStatement(tok->astParent()); - return !isNestedBracket && isConstStatement(tok->astOperand2()); + return isNestedBracket && isConstStatement(tok->astOperand2()); } return isConstStatement(tok->astOperand2(), /*isNestedBracket*/ !isChained); }