removeContradiction() Avoid use-after-free on multiple remove

As reported in https://sourceforge.net/p/cppcheck/discussion/general/thread/fa43fb8ab1/ removeContradiction() minValue/maxValue.remove(..) can access free'd memory as it removes all matching values by iterating over the complete list. Creating a full copy instead of a reference avoids this issue. Signed-off-by: Dirk Müller <[email protected]>
danmar · Nov 28, 2023 · 770d4b6 · 770d4b6
1 parent db66105
commit 770d4b6
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 3 deletions.
diff --git a/lib/token.cpp b/lib/token.cpp
@@ -1997,8 +1997,8 @@ static bool removeContradiction(std::list<ValueFlow::Value>& values)
                 auto compare = [](const ValueFlow::Value& x, const ValueFlow::Value& y) {
                     return x.compareValue(y, less{});
                 };
-                const ValueFlow::Value& maxValue = std::max(x, y, compare);
-                const ValueFlow::Value& minValue = std::min(x, y, compare);
+                ValueFlow::Value maxValue = std::max(x, y, compare);
+                ValueFlow::Value minValue = std::min(x, y, compare);
                 // TODO: Adjust non-points instead of removing them
                 if (maxValue.isImpossible() && maxValue.bound == ValueFlow::Value::Bound::Upper) {
                     values.remove(minValue);

diff --git a/test/testgarbage.cpp b/test/testgarbage.cpp
@@ -252,6 +252,7 @@ class TestGarbage : public TestFixture {
         TEST_CASE(garbageCode221);
         TEST_CASE(garbageCode222); // #10763
         TEST_CASE(garbageCode223); // #11639
+        TEST_CASE(garbageCode224);
 
         TEST_CASE(garbageCodeFuzzerClientMode1); // test cases created with the fuzzer client, mode 1
 
@@ -1719,7 +1720,11 @@ class TestGarbage : public TestFixture {
     void garbageCode223() { // #11639
         ASSERT_THROW(checkCode("struct{}*"), InternalError);  // don't crash
     }
-
+    void garbageCode224() {
+        // don't crash
+        checkCode("8&88888tdio:h>\n"
+                  "Jint m");
+    }
     void syntaxErrorFirstToken() {
         ASSERT_THROW(checkCode("&operator(){[]};"), InternalError); // #7818
         ASSERT_THROW(checkCode("*(*const<> (size_t); foo) { } *(*const (size_t)() ; foo) { }"), InternalError); // #6858