From 3deff9c578381edabf79f9800c4d472cd199f3bb Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 6 Jun 2019 18:22:36 +0100
Subject: [PATCH] CPP: Fix in dataflow.

---
 cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll       | 6 +++++-
 .../TaintedAllocationSize/TaintedAllocationSize.expected    | 1 -
 .../CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp       | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll
index 4ba37e20854..6bbc3a416cd 100644
--- a/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/TaintTracking.qll
@@ -75,7 +75,11 @@ predicate moveToDependingOnSide(Expr src, Expr dest) {
     and (base = binop.getLeftOperand() or base = binop.getRightOperand())
     and isPointer(base.getType())
     and base.getTarget() instanceof LocalScopeVariable
-    and src = base)
+    and src = base
+
+    // flow through pointer-pointer subtraction is dubious, the result should be
+    // a number bounded by the size of the pointed-to thing.
+    and not binop instanceof PointerDiffExpr)
   or exists (UnaryOperation unop |
     dest = unop
     and unop.getAnOperand() = src)
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected
index ebd736014bb..ec6ac0eade7 100644
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected
@@ -4,4 +4,3 @@
 | test.cpp:49:17:49:30 | new[] | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
 | test.cpp:52:35:52:60 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
 | test.cpp:55:11:55:24 | new[] | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
-| test.cpp:79:9:79:29 | new[] | This allocation size is derived from $@ and might overflow | test.cpp:97:18:97:23 | buffer | user input (fread) |
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp
index bd0ad7f75a0..0722d95cf46 100644
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp
@@ -76,7 +76,7 @@ void processData2(char *start, char *end)
 {
 	char *copy;
 
-	copy = new char[end - start]; // GOOD [FALSE POSITIVE]
+	copy = new char[end - start]; // GOOD
 
 	// ...