github · hvitved · Nov 13, 2018 · Oct 15, 2018 · Oct 17, 2018 · Oct 17, 2018
diff --git a/change-notes/1.19/analysis-cpp.md b/change-notes/1.19/analysis-cpp.md
@@ -16,6 +16,7 @@
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Empty branch of conditional | Fewer false positive results | The query now recognizes commented blocks more reliably. |
+| Expression has no effect | Fewer false positive results | Expressions in template instantiations are now excluded from this query. |
 | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. Also fixed an issue where false positives could occur if the destructor body was not in the snapshot. |
 | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. |
 | Missing return statement | Fewer false positive results | The query is now produces correct results when a function returns a template-dependent type. |

diff --git a/change-notes/1.19/analysis-csharp.md b/change-notes/1.19/analysis-csharp.md
@@ -10,7 +10,8 @@
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
-| *@name of query (Query ID)* | *Tags*    |*Aim of the new query and whether it is enabled by default or not*  |
+| Using a package with a known vulnerability (cs/use-of-vulnerable-package) | security, external/cwe/cwe-937 | Finds project build files that import packages with known vulnerabilities. This is included by default. |
+
 
 ## Changes to existing queries
 

diff --git a/change-notes/1.19/analysis-javascript.md b/change-notes/1.19/analysis-javascript.md
@@ -41,6 +41,7 @@
 | Unused import | Fewer false-positive results | This rule no longer flags imports used by the `transform-react-jsx` Babel plugin. |
 | Self assignment | Fewer false-positive results | This rule now ignores self-assignments preceded by a JSDoc comment with a `@type` tag. |
 | Client side cross-site scripting | More results | This rule now also flags HTML injection in the body of an email. |
+| Information exposure through a stack trace | More results | This rule now also flags cases where the entire exception object (including the stack trace) may be exposed. |
 
 ## Changes to QL libraries
 

diff --git a/cpp/ql/src/Critical/InconsistentNullnessTesting.cpp b/cpp/ql/src/Critical/InconsistentNullnessTesting.cpp
@@ -1,10 +1,10 @@
 void* f() {
-	block = malloc(BLOCK_SIZE);
+	block = (MyBlock *)malloc(sizeof(MyBlock));
 	if (block) { //correct: block is checked for nullness here
 		block->id = NORMAL_BLOCK_ID;
 	}
 	//...
 	/* make sure data-portion is null-terminated */
-	block[BLOCK_SIZE - 1] = '\0'; //wrong: block not checked for nullness here
+	block->data[BLOCK_SIZE - 1] = '\0'; //wrong: block not checked for nullness here
 	return block;
 }
diff --git a/cpp/ql/src/Critical/OverflowCalculated.cpp b/cpp/ql/src/Critical/OverflowCalculated.cpp
@@ -1,5 +1,5 @@
 void f(char* string) {
-	// wrong: allocates space for characters, put not zero terminator
+	// wrong: allocates space for characters, but not zero terminator
 	char* buf = malloc(strlen(string));
 	strcpy(buf, string);
 

diff --git a/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql b/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql
@@ -100,6 +100,7 @@ where // EQExprs are covered by CompareWhereAssignMeant.ql
       not accessInInitOfForStmt(peivc) and
       not peivc.isCompilerGenerated() and
       not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and
+      not peivc.isFromTemplateInstantiation(_) and
       parent = peivc.getParent() and
       not parent.isInMacroExpansion() and
       not parent instanceof PureExprInVoidContext and

diff --git a/cpp/ql/test/format.json b/cpp/ql/test/format.json
@@ -0,0 +1,11 @@
+[
+  {
+    "pattern": [
+      "**/*.c",
+      "**/*.cpp",
+      "**/*.h",
+      "**/*.hpp"
+    ],
+    "allowMixedTabsAndSpaces": true
+  }
+]
diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp b/cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp
@@ -5,26 +5,26 @@ int external();
 class Base {
 public:
   virtual int thingy() {
-    1;
+    1; // BAD
   }
 
   int our_thingy() {
-    Base::thingy();
+    Base::thingy(); // BAD
     return 2;
   }
 };
 
 class Derived : public Base {
 public:
   virtual int thingy() {
-    external();
+    external(); // GOOD
     return 3;
   }
 };
 
 void internal() {
   Base* ptr = new Derived();
-  ptr->thingy();
+  ptr->thingy(); // GOOD
 }
 
 }
diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected b/cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected
@@ -1,6 +1,5 @@
 | calls.cpp:8:5:8:5 | 1 | This expression has no effect. | calls.cpp:8:5:8:5 | 1 |  |
 | calls.cpp:12:5:12:16 | call to thingy | This expression has no effect (because $@ has no external side effects). | calls.cpp:7:15:7:20 | thingy | thingy |
-| templatey.cpp:4:3:4:8 | ... << ... | This expression has no effect. | templatey.cpp:4:3:4:8 | ... << ... |  |
 | templatey.cpp:39:3:39:23 | call to pointless_add_numbers | This expression has no effect (because $@ has no external side effects). | templatey.cpp:29:5:29:25 | pointless_add_numbers | pointless_add_numbers |
 | volatile.c:9:5:9:5 | c | This expression has no effect. | volatile.c:9:5:9:5 | c |  |
 | volatile.c:12:5:12:9 | access to array | This expression has no effect. | volatile.c:12:5:12:9 | access to array |  |

diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp b/cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp
@@ -1,7 +1,7 @@
 template <typename T>
 void foo(T x, T y)
 {
-  x << y;
+  x << y; // GOOD (effect depends on T)
 };
 
 struct streamable
@@ -15,9 +15,9 @@ void operator<<(streamable& lhs, streamable& rhs)
 int main()
 {
   int x = 3;
-  foo(x, x);
+  foo(x, x); // BAD [NOT DETECTED]
   streamable y;
-  foo(y, y);
+  foo(y, y); // BAD [NOT DETECTED]
   return 0;
 }
 
@@ -34,7 +34,7 @@ int pointless_add_numbers(int lhs, int rhs)
 void call_add_numbers()
 {
   int accum = 0;
-  add_numbers(accum, 4);
-  add_numbers(accum, 10);
-  pointless_add_numbers(accum, 20);
+  add_numbers(accum, 4); // GOOD
+  add_numbers(accum, 10); // GOOD
+  pointless_add_numbers(accum, 20); // BAD
 }
diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c b/cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c
@@ -6,18 +6,18 @@ char *pc;
 volatile char *pv;
 
 void f(void) {
-    c;
-    v;
+    c; // BAD
+    v; // (accesses to volatile variables are considered impure)
 
-    pc[5];
+    pc[5]; // BAD
     pv[5];
     ((volatile char *)pc)[5];
 
-    *pc;
+    *pc; // BAD
     *pv;
     *((volatile char *)pc);
 
-    *(pc + 5);
+    *(pc + 5); // BAD
     *(pv + 5);
     *((volatile char *)(pc + 5));
     *(((volatile char *)pc) + 5);

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected
@@ -1,5 +1,6 @@
 | preproc.c:89:2:89:4 | call to fn4 | This expression has no effect (because $@ has no external side effects). | preproc.c:33:5:33:7 | fn4 | fn4 |
 | preproc.c:94:2:94:4 | call to fn9 | This expression has no effect (because $@ has no external side effects). | preproc.c:78:5:78:7 | fn9 | fn9 |
+| template.cpp:19:3:19:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ |
 | test.c:7:5:7:5 | 0 | This expression has no effect. | test.c:7:5:7:5 | 0 |  |
 | test.c:9:8:9:8 | 1 | This expression has no effect. | test.c:9:8:9:8 | 1 |  |
 | test.c:9:11:9:11 | 2 | This expression has no effect. | test.c:9:11:9:11 | 2 |  |
@@ -19,11 +20,6 @@
 | test.c:26:15:26:16 | 32 | This expression has no effect. | test.c:26:15:26:16 | 32 |  |
 | test.c:27:9:27:10 | 33 | This expression has no effect. | test.c:27:9:27:10 | 33 |  |
 | test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
 | test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:26:3:26:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
 | test.cpp:62:5:62:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:47:14:47:22 | operator= | operator= |
 | test.cpp:65:5:65:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:55:7:55:7 | operator= | operator= |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c
@@ -0,0 +1,7 @@
+
+#define UNUSED(x) (x)
+
+void test2(int param)
+{
+	UNUSED(param); // GOOD
+}
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp
@@ -0,0 +1,22 @@
+
+template<class T>
+void Increment(T &t) {
+	t++; // GOOD (sometimes has an effect)
+}
+
+class Nothing {
+public:
+	Nothing operator++(int) {
+		return *this; // do nothing
+	}
+};
+
+void myTemplateTest() {
+	int i = 0;
+	Nothing n;
+
+	i++; // GOOD (always has an effect)
+	n++; // BAD (never has an effect)
+	Increment(i);
+	Increment(n);
+}
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp
@@ -23,7 +23,7 @@ class MyTemplateClass
 
 		++arg1; // pure, does nothing
 		++arg2; // pure, does nothing
-		++arg3; // not pure in all cases (when _It is int this has a side-effect) [FALSE POSITIVE]
+		++arg3; // not pure in all cases (when _It is int this has a side-effect)
 
 		return arg2;
 	}