diff --git a/change-notes/1.20/analysis-cpp.md b/change-notes/1.20/analysis-cpp.md index fa8ce23f5b71..3bf96b8b9fb4 100644 --- a/change-notes/1.20/analysis-cpp.md +++ b/change-notes/1.20/analysis-cpp.md @@ -11,6 +11,7 @@ | **Query** | **Expected impact** | **Change** | |----------------------------|------------------------|------------------------------------------------------------------| +| Suspicious pointer scaling (`cpp/suspicious-pointer-scaling`) | Fewer false positives | False positives involving types that are not uniquely named in the snapshot have been fixed. | | Unused static variable (`cpp/unused-static-variable`) | Fewer false positive results | Variables with the attribute `unused` are now excluded from the query. | ## Changes to QL libraries diff --git a/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql b/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql index 6417d025e6cf..fc542790e22f 100644 --- a/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql +++ b/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql @@ -34,7 +34,6 @@ private Type baseType(Type t) { ) // Make sure that the type has a size and that it isn't ambiguous. and strictcount(result.getSize()) = 1 - } /** @@ -98,6 +97,7 @@ predicate defSourceType(SsaDefinition def, LocalScopeVariable v, | p = v and def.definedByParameter(p) and sourceType = p.getType().getUnspecifiedType() and + strictcount(p.getType()) = 1 and isPointerType(sourceType) and sourceLoc = p.getLocation()) } diff --git a/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingChar.ql b/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingChar.ql index fa921e2beb42..c87f78c07eb4 100644 --- a/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingChar.ql +++ b/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingChar.ql @@ -89,6 +89,7 @@ predicate defSourceType(SsaDefinition def, LocalScopeVariable v, | p = v and def.definedByParameter(p) and sourceType = p.getType().getUnspecifiedType() and + strictcount(p.getType()) = 1 and isPointerType(sourceType) and sourceLoc = p.getLocation()) } diff --git a/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingVoid.ql b/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingVoid.ql index c91c9a83e9f1..bcd76ca1790f 100644 --- a/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingVoid.ql +++ b/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingVoid.ql @@ -89,6 +89,7 @@ predicate defSourceType(SsaDefinition def, LocalScopeVariable v, | p = v and def.definedByParameter(p) and sourceType = p.getType().getUnspecifiedType() and + strictcount(p.getType()) = 1 and isPointerType(sourceType) and sourceLoc = p.getLocation()) } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/IncorrectPointerScaling/test_large.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/IncorrectPointerScaling/test_large.cpp new file mode 100644 index 000000000000..6431f1ec1fca --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/IncorrectPointerScaling/test_large.cpp @@ -0,0 +1,10 @@ + +struct MyStruct +{ + int x, y, z, w; +}; + +void test(MyStruct *ptr) +{ + MyStruct *new_ptr = ptr + 1; // GOOD +} diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/IncorrectPointerScaling/test_small.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/IncorrectPointerScaling/test_small.cpp new file mode 100644 index 000000000000..4740fd6db2b0 --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/IncorrectPointerScaling/test_small.cpp @@ -0,0 +1,13 @@ +// note the two different `MyStruct` definitions, in test_small.cpp and test_large.cpp. These are +// in different translation units and we assume they are never linked into the same program (which +// would result in undefined behaviour). + +struct MyStruct +{ + int x, y; +}; + +void test(MyStruct *ptr) +{ + MyStruct *new_ptr = ptr + 1; // GOOD +}