From 24a63ae94de387bd61d0f07d67d81d9833f47511 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 15 Feb 2024 10:17:20 +0100 Subject: [PATCH] C++: Block flow by default. --- .../cpp/ir/dataflow/internal/SsaInternals.qll | 42 +++++++++++++++++- .../dataflow-tests/test-source-sink.expected | 2 - .../dataflow/dataflow-tests/test.cpp | 4 +- .../dataflow/taint-tests/map.cpp | 44 +++++++++---------- .../dataflow/taint-tests/set.cpp | 32 +++++++------- .../dataflow/taint-tests/string.cpp | 8 ++-- .../dataflow/taint-tests/stringstream.cpp | 14 +++--- .../dataflow/taint-tests/taint.cpp | 4 +- .../dataflow/taint-tests/vector.cpp | 6 +-- 9 files changed, 97 insertions(+), 59 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 8a5e8d20319..a6255abcee3 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -4,7 +4,10 @@ private import DataFlowUtil private import DataFlowImplCommon as DataFlowImplCommon private import semmle.code.cpp.models.interfaces.Allocation as Alloc private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow +private import semmle.code.cpp.models.interfaces.Taint as Taint +private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as FIO private import semmle.code.cpp.ir.internal.IRCppLanguage +private import semmle.code.cpp.ir.dataflow.internal.ModelUtil private import DataFlowPrivate private import ssa0.SsaInternals as SsaInternals0 import SsaInternalsCommon @@ -796,10 +799,47 @@ private Node getAPriorDefinition(SsaDefOrUse defOrUse) { ) } +private predicate inOut(FIO::FunctionInput input, FIO::FunctionOutput output) { + exists(int indirectionIndex | + input.isQualifierObject(indirectionIndex) and + output.isQualifierObject(indirectionIndex) + or + exists(int i | + input.isParameterDeref(i, indirectionIndex) and + output.isParameterDeref(i, indirectionIndex) + ) + ) +} + +/** + * Holds if there should not be use-use flow out of `n` (or a conversion that + * flows to `n`). + */ +private predicate modeledFlowBarrier(Node n) { + exists(FIO::FunctionInput input, FIO::FunctionOutput output, CallInstruction call | + n = callInput(call, input) and + inOut(input, output) and + exists(callOutput(call, output)) + | + call.getStaticCallTarget().(DataFlow::DataFlowFunction).hasDataFlow(_, output) + or + call.getStaticCallTarget().(Taint::TaintFunction).hasTaintFlow(_, output) + ) + or + exists(Operand operand, Instruction instr, Node n0, int indirectionIndex | + modeledFlowBarrier(n0) and + nodeHasInstruction(n0, instr, indirectionIndex) and + conversionFlow(operand, instr, false, _) and + nodeHasOperand(n, operand, indirectionIndex) + ) +} + /** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */ predicate ssaFlow(Node nodeFrom, Node nodeTo) { exists(Node nFrom, boolean uncertain, SsaDefOrUse defOrUse | - ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and nodeFrom != nodeTo + ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and + not modeledFlowBarrier(nFrom) and + nodeFrom != nodeTo | if uncertain = true then nodeFrom = [nFrom, getAPriorDefinition(defOrUse)] else nodeFrom = nFrom ) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected index 2d33f47ba60..f9ccfb8e3e4 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected @@ -238,8 +238,6 @@ irFlow | test.cpp:382:48:382:54 | source1 | test.cpp:385:8:385:10 | tmp | | test.cpp:388:53:388:59 | source1 | test.cpp:392:8:392:10 | tmp | | test.cpp:388:53:388:59 | source1 | test.cpp:394:10:394:12 | tmp | -| test.cpp:399:7:399:9 | definition of tmp | test.cpp:401:8:401:10 | tmp | -| test.cpp:405:7:405:9 | definition of tmp | test.cpp:408:8:408:10 | tmp | | test.cpp:416:7:416:11 | definition of local | test.cpp:418:8:418:12 | local | | test.cpp:417:16:417:20 | intRefSource output argument | test.cpp:418:8:418:12 | local | | test.cpp:422:7:422:11 | definition of local | test.cpp:424:8:424:12 | local | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index e29619a6800..60c3abfdfc6 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -398,14 +398,14 @@ void flowThroughMemcpy_blockvar_with_local_flow(int source1, int b) { void cleanedByMemcpy_ssa(int clean1) { // currently modeled with BlockVar, not SSA int tmp; memcpy(&tmp, &clean1, sizeof tmp); - sink(tmp); // $ SPURIOUS: ast,ir + sink(tmp); // $ SPURIOUS: ast } void cleanedByMemcpy_blockvar(int clean1) { int tmp; int *capture = &tmp; memcpy(&tmp, &clean1, sizeof tmp); - sink(tmp); // $ SPURIOUS: ast,ir + sink(tmp); // $ SPURIOUS: ast } void intRefSource(int &ref_source); diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp index 8eeb80a0f83..9e361e9cf49 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp @@ -71,11 +71,11 @@ void test_pair() sink(i.second); // $ MISSING: ast,ir sink(i); // $ ast,ir sink(j.first); - sink(j.second); // $ SPURIOUS: ast,ir - sink(j); // $ SPURIOUS: ast,ir + sink(j.second); // $ SPURIOUS: ast + sink(j); // $ SPURIOUS: ast sink(k.first); - sink(k.second); // $ SPURIOUS: ast,ir - sink(k); // $ SPURIOUS: ast,ir + sink(k.second); // $ SPURIOUS: ast + sink(k); // $ SPURIOUS: ast sink(l.first); sink(l.second); // $ MISSING: ast,ir sink(l); // $ ast,ir @@ -179,11 +179,11 @@ void test_map() m14.insert(std::make_pair("b", source())); m14.insert(std::make_pair("c", source())); m14.insert(std::make_pair("d", "d")); - sink(m14.lower_bound("b")); // $ ast,ir=179:33 ast,ir=180:33 - sink(m14.upper_bound("b")); // $ ast,ir=179:33 ast,ir=180:33 + sink(m14.lower_bound("b")); // $ ast=179:33 ast=180:33 MISSING: ir=179:33 ir=180:33 + sink(m14.upper_bound("b")); // $ ast=179:33 ast=180:33 MISSING: ir=179:33 ir=180:33 sink(m14.equal_range("b").first); // $ MISSING: ast,ir sink(m14.equal_range("b").second); // $ MISSING: ast,ir - sink(m14.upper_bound("c")); // $ SPURIOUS: ast,ir=179:33 ast,ir=180:33 + sink(m14.upper_bound("c")); // $ SPURIOUS: ast=179:33 ast=180:33 sink(m14.equal_range("c").second); // swap @@ -196,10 +196,10 @@ void test_map() sink(m18); // $ ast,ir m15.swap(m16); m17.swap(m18); - sink(m15); // $ SPURIOUS: ast,ir + sink(m15); // $ SPURIOUS: ast sink(m16); // $ ast,ir sink(m17); // $ ast,ir - sink(m18); // $ SPURIOUS: ast,ir + sink(m18); // $ SPURIOUS: ast // merge std::map m19, m20, m21, m22; @@ -213,7 +213,7 @@ void test_map() sink(m22); // $ ast,ir m19.merge(m20); m21.merge(m22); - sink(m19); // $ ast,ir + sink(m19); // $ ast sink(m20); sink(m21); // $ ast,ir sink(m22); // $ ast,ir @@ -222,11 +222,11 @@ void test_map() std::map m23; m23.insert(std::pair(source(), source())); m23.insert(std::pair(source(), source())); - sink(m23); // $ ast,ir=223:49 ast,ir=224:49 - sink(m23.erase(m23.begin())); // $ ast,ir=223:49 ast,ir=224:49 - sink(m23); // $ ast,ir=223:49 ast,ir=224:49 + sink(m23); // $ ast=223:49 ast=224:49 ir MISSING: ir=223:49 ir=224:49 + sink(m23.erase(m23.begin())); // $ ast=223:49 ast=224:49 ir MISSING: ir=223:49 ir=224:49 + sink(m23); // $ ast=223:49 ast=224:49 ir MISSING: ir=223:49 ir=224:49 m23.clear(); - sink(m23); // $ SPURIOUS: ast,ir=223:49 ast,ir=224:49 + sink(m23); // $ SPURIOUS: ast=223:49 ast=224:49 ir // emplace, emplace_hint std::map m24, m25; @@ -345,10 +345,10 @@ void test_unordered_map() sink(m18); // $ ast,ir m15.swap(m16); m17.swap(m18); - sink(m15); // $ SPURIOUS: ast,ir + sink(m15); // $ SPURIOUS: ast sink(m16); // $ ast,ir sink(m17); // $ ast,ir - sink(m18); // $ SPURIOUS: ast,ir + sink(m18); // $ SPURIOUS: ast // merge std::unordered_map m19, m20, m21, m22; @@ -362,7 +362,7 @@ void test_unordered_map() sink(m22); // $ ast,ir m19.merge(m20); m21.merge(m22); - sink(m19); // $ ast,ir + sink(m19); // $ ast MISSING: ir sink(m20); sink(m21); // $ ast,ir sink(m22); // $ ast,ir @@ -371,11 +371,11 @@ void test_unordered_map() std::unordered_map m23; m23.insert(std::pair(source(), source())); m23.insert(std::pair(source(), source())); - sink(m23); // $ ast,ir=372:49 ast,ir=373:49 - sink(m23.erase(m23.begin())); // $ ast,ir=372:49 ast,ir=373:49 - sink(m23); // $ ast,ir=372:49 ast,ir=373:49 + sink(m23); // $ ast=372:49 ast=373:49 ir MISSING: ir=372:49 ir=373:49 + sink(m23.erase(m23.begin())); // $ ast=372:49 ast=373:49 ir MISSING: ir=372:49 ir=373:49 + sink(m23); // $ ast=372:49 ast=373:49 ir MISSING: ir=372:49 ir=373:49 m23.clear(); - sink(m23); // $ SPURIOUS: ast,ir=372:49 ast,ir=373:49 + sink(m23); // $ SPURIOUS: ast=372:49 ast=373:49 ir // emplace, emplace_hint std::unordered_map m24, m25; @@ -395,7 +395,7 @@ void test_unordered_map() sink(m26); sink(m26.try_emplace("abc", source()).first); sink(m26.try_emplace("abc", source()).second); // $ MISSING: ast,ir=396:30 - sink(m26); // $ ast,ir=396:30 SPURIOUS: ast,ir=397:30 + sink(m26); // $ ast=396:30 ir MISSING: ir=396:30 SPURIOUS: ast=397:30 sink(m27.try_emplace(m27.begin(), "abc", "def")); sink(m27); sink(m27.try_emplace(m27.begin(), "abc", source())); // $ ast,ir diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp index c6c19d90089..0715eac8b4e 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp @@ -66,8 +66,8 @@ void test_set() s11.insert("a"); s11.insert(source()); s11.insert("c"); - sink(s11.lower_bound("b")); // $ ast,ir - sink(s11.upper_bound("b")); // $ ast,ir + sink(s11.lower_bound("b")); // $ ast MISSING: ir + sink(s11.upper_bound("b")); // $ ast MISSING: ir sink(s11.equal_range("b").first); // $ MISSING: ast,ir sink(s11.equal_range("b").second); // $ MISSING: ast,ir @@ -81,10 +81,10 @@ void test_set() sink(s15); // $ ast,ir s12.swap(s13); s14.swap(s15); - sink(s12); // $ SPURIOUS: ast,ir + sink(s12); // $ SPURIOUS: ast sink(s13); // $ ast,ir sink(s14); // $ ast,ir - sink(s15); // $ SPURIOUS: ast,ir + sink(s15); // $ SPURIOUS: ast // merge std::set s16, s17, s18, s19; @@ -98,7 +98,7 @@ void test_set() sink(s19); // $ ast,ir s16.merge(s17); s18.merge(s19); - sink(s16); // $ ast,ir + sink(s16); // $ ast MISSING: ir sink(s17); sink(s18); // $ ast,ir sink(s19); // $ ast,ir @@ -107,11 +107,11 @@ void test_set() std::set s20; s20.insert(source()); s20.insert(source()); - sink(s20); // $ ast,ir=108:13 ast,ir=109:13 - sink(s20.erase(s20.begin())); // $ ast,ir=108:13 ast,ir=109:13 - sink(s20); // $ ast,ir=108:13 ast,ir=109:13 + sink(s20); // $ ast=108:13 ast=109:13 ir MISSING: ir=108:13 ir=109:13 + sink(s20.erase(s20.begin())); // $ ast=108:13 ast=109:13 ir MISSING: ir=108:13 ir=109:13 + sink(s20); // $ ir ast=108:13 ast=109:13 MISSING: ir=108:13 ir=109:13 s20.clear(); - sink(s20); // $ SPURIOUS: ast,ir=108:13 ast,ir=109:13 + sink(s20); // $ SPURIOUS: ir ast=108:13 ast=109:13 // emplace, emplace_hint std::set s21, s22; @@ -193,10 +193,10 @@ void test_unordered_set() sink(s15); // $ ast,ir s12.swap(s13); s14.swap(s15); - sink(s12); // $ SPURIOUS: ast,ir + sink(s12); // $ SPURIOUS: ast sink(s13); // $ ast,ir sink(s14); // $ ast,ir - sink(s15); // $ SPURIOUS: ast,ir + sink(s15); // $ SPURIOUS: ast // merge std::unordered_set s16, s17, s18, s19; @@ -210,7 +210,7 @@ void test_unordered_set() sink(s19); // $ ast,ir s16.merge(s17); s18.merge(s19); - sink(s16); // $ ast,ir + sink(s16); // $ ast MISSING: ir sink(s17); sink(s18); // $ ast,ir sink(s19); // $ ast,ir @@ -219,11 +219,11 @@ void test_unordered_set() std::unordered_set s20; s20.insert(source()); s20.insert(source()); - sink(s20); // $ ast,ir=220:13 ast,ir=221:13 - sink(s20.erase(s20.begin())); // $ ast,ir=220:13 ast,ir=221:13 - sink(s20); // $ ast,ir=220:13 ast,ir=221:13 + sink(s20); // $ ir ast=220:13 ast=221:13 MISSING: ir=220:13 ir=221:13 + sink(s20.erase(s20.begin())); // $ ast=220:13 ast=221:13 ir MISSING: ir=220:13 ir=221:13 + sink(s20); // $ ast=220:13 ast=221:13 ir MISSING: ir=220:13 ir=221:13 s20.clear(); - sink(s20); // $ SPURIOUS: ast,ir=220:13 ast,ir=221:13 + sink(s20); // $ SPURIOUS: ast=220:13 ast=221:13 ir // emplace, emplace_hint std::unordered_set s21, s22; diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp index e2b99945724..dc92a0664be 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp @@ -203,7 +203,7 @@ void test_string_assign() { sink(s5); // $ ast,ir sink(s6.assign(s1)); - sink(s6); // $ SPURIOUS: ast,ir + sink(s6); // $ SPURIOUS: ast } void test_string_insert() { @@ -280,9 +280,9 @@ void test_string_swap() { s4.swap(s3); sink(s1); // $ ast,ir - sink(s2); // $ SPURIOUS: ast,ir + sink(s2); // $ SPURIOUS: ast sink(s3); // $ ast,ir - sink(s4); // $ SPURIOUS: ast,ir + sink(s4); // $ SPURIOUS: ast } void test_string_clear() { @@ -495,7 +495,7 @@ void test_string_iterator_methods() sink(h); // $ ast,ir sink(s6.assign(s5.cbegin(), s5.cend())); - sink(s6); // $ SPURIOUS: ast,ir + sink(s6); // $ SPURIOUS: ast } } diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp index a84b3606f92..ca17fb4b3e7 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp @@ -50,7 +50,7 @@ void test_stringstream_string(int amount) ss7.str(source()); ss7.str("abc"); // (overwrites) sink(ss6); // $ ast,ir - sink(ss7); // $ SPURIOUS: ast,ir + sink(ss7); // $ SPURIOUS: ast sink(ss8.put('a')); sink(ss9.put(ns_char::source())); // $ ast,ir @@ -118,9 +118,9 @@ void test_stringstream_swap() ss4.swap(ss3); sink(ss1); // $ ast,ir - sink(ss2); // $ SPURIOUS: ast,ir + sink(ss2); // $ SPURIOUS: ast sink(ss3); // $ ast,ir - sink(ss4); // $ SPURIOUS: ast,ir + sink(ss4); // $ SPURIOUS: ast } void test_stringstream_in() @@ -217,7 +217,7 @@ void test_getline() sink(ss1.getline(b3, 1000)); sink(b1); sink(b2); // $ ast,ir - sink(b3); // $ SPURIOUS: ast,ir + sink(b3); // $ SPURIOUS: ast sink(ss1.getline(b4, 1000, ' ')); sink(ss2.getline(b5, 1000, ' ')); // $ ast,ir @@ -225,7 +225,7 @@ void test_getline() sink(ss1.getline(b6, 1000, ' ')); sink(b4); sink(b5); // $ ast,ir - sink(b6); // $ SPURIOUS: ast,ir + sink(b6); // $ SPURIOUS: ast sink(ss2.getline(b7, 1000).getline(b8, 1000)); // $ ast,ir sink(b7); // $ ast,ir @@ -237,7 +237,7 @@ void test_getline() sink(getline(ss1, s3)); sink(s1); sink(s2); // $ ast,ir - sink(s3); // $ SPURIOUS: ast,ir + sink(s3); // $ SPURIOUS: ast sink(getline(ss1, s4, ' ')); sink(getline(ss2, s5, ' ')); // $ ast,ir @@ -245,7 +245,7 @@ void test_getline() sink(getline(ss1, s6, ' ')); sink(s4); sink(s5); // $ ast,ir - sink(s6); // $ SPURIOUS: ast,ir + sink(s6); // $ SPURIOUS: ast sink(getline(getline(ss2, s7), s8)); // $ ast,ir sink(s7); // $ ast,ir diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp index 02ddc44d883..e24830a3004 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -212,7 +212,7 @@ void test_swap() { std::swap(x, y); - sink(x); // $ SPURIOUS: ast,ir + sink(x); // $ SPURIOUS: ast sink(y); // $ ast,ir } @@ -756,5 +756,5 @@ void call_sprintf_twice(char* path, char* data) { void test_call_sprintf() { char path[10]; call_sprintf_twice(path, indirect_source()); - sink(*path); // $ ir ast + sink(*path); // $ ast MISSING: ir } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp index a26ac8f0513..2728be23e2e 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp @@ -114,10 +114,10 @@ void test_vector_swap() { v1.swap(v2); v3.swap(v4); - sink(v1); // $ SPURIOUS: ast,ir + sink(v1); // $ SPURIOUS: ast sink(v2); // $ ast,ir sink(v3); // $ ast,ir - sink(v4); // $ SPURIOUS: ast,ir + sink(v4); // $ SPURIOUS: ast } void test_vector_clear() { @@ -138,7 +138,7 @@ void test_vector_clear() { sink(v1); // $ SPURIOUS: ast,ir sink(v2); // $ ast,ir - sink(v3); // $ ast,ir + sink(v3); // $ SPURIOUS: ast sink(v4); }