Merge pull request #644 from geoffw0/pointerscaling

CPP: Fix type confusion in IncorrectPointerscaling.ql

Author: jbj

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

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())
 }

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())
 }

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())
 }

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
+}

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
+}