github · semmle-qlci · Nov 1, 2018 · Nov 8, 2018 · Nov 8, 2018 · Nov 8, 2018
diff --git a/change-notes/1.19/analysis-csharp.md b/change-notes/1.19/analysis-csharp.md
@@ -20,7 +20,12 @@
 | Cross-site scripting (`cs/web/xss`) | More results | This query now finds cross-site scripting vulnerabilities in ASP.NET Core applications. |
 | *@name of query (Query ID)*| *Impact on results*    | *How/why the query has changed*                                  |
 
+## Changes to code extraction
+
+* Arguments passed using `in` are now extracted.
+* Fix a bug where the `dynamic` type name was not extracted correctly in certain circumstances.
 
 ## Changes to QL libraries
 
 * `getArgument()` on `AccessorCall` has been improved so it now takes tuple assignments into account. For example, the argument for the implicit `value` parameter in the setter of property `P` is `0` in `(P, x) = (0, 1)`. Additionally, the argument for the `value` parameter in compound assignments is now only the expanded value, for example, in `P += 7` the argument is `P + 7` and not `7`.
+* The predicate `isInArgument()` has been added to the `AssignableAccess` class. This holds for expressions that are passed as arguments using `in`.
diff --git a/change-notes/1.19/analysis-java.md b/change-notes/1.19/analysis-java.md
@@ -2,6 +2,8 @@
 
 ## General improvements
 
+* Where applicable, path explanations have been added to the security queries.
+
 ## New queries
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |

diff --git a/change-notes/1.19/analysis-javascript.md b/change-notes/1.19/analysis-javascript.md
@@ -6,13 +6,17 @@
 
 * The taint tracking library now recognizes additional sanitization patterns. This may give fewer false-positive results for the security queries.
 
+* Support for AMD modules has been improved. This may give additional results for the security queries as well as any queries that use type inference on code bases that use such modules.
+
 * Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features:
   - file system access, for example through [fs-extra](https://github.com/jprichardson/node-fs-extra) or [globby](https://www.npmjs.com/package/globby)
   - outbound network access, for example through the [fetch API](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API)
   - the [lodash](https://lodash.com), [underscore](https://underscorejs.org/), [async](https://www.npmjs.com/package/async) and [async-es](https://www.npmjs.com/package/async-es) libraries
 
 * Type inference for function calls has been improved. This may give additional results for queries that rely on type inference.
 
+* Where applicable, path explanations have been added to the security queries.
+
 ## New queries
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |
@@ -45,6 +49,8 @@
 | Unused variable, import, function or class | Fewer results | This rule now flags import statements with multiple unused imports once. |
 | Useless assignment to local variable | Fewer false-positive results | This rule now recognizes additional ways default values can be set. |
 | Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
+| Client-side URL redirect | Fewer false-positive results | This rule now recognizes safe redirects in more cases. |
+| Server-side URL redirect | Fewer false-positive results | This rule now recognizes safe redirects in more cases. |
 
 ## Changes to QL libraries
 

diff --git a/cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.qhelp b/cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.qhelp
diff --git a/cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.ql b/cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.ql
@@ -1,6 +1,6 @@
 /**
  * @name Class hierarchies
- * @description Shows classes and their base classes.
+ * @description Shows an inheritance hierarchy for classes and their base classes.
  * @kind graph
  * @id cpp/architecture/class-hierarchies
  * @graph.layout organic

diff --git a/cpp/ql/src/Architecture/General Class-Level Information/HubClasses.qhelp b/cpp/ql/src/Architecture/General Class-Level Information/HubClasses.qhelp
diff --git a/cpp/ql/src/Architecture/General Class-Level Information/HubClasses.ql b/cpp/ql/src/Architecture/General Class-Level Information/HubClasses.ql
@@ -1,6 +1,6 @@
 /**
  * @name Hub classes
- * @description Shows coupling between classes; red, large boxes are hub types that depend on many other classes
+ * @description Shows coupling between classes. Large, red, boxes are hub types that depend on many other classes
  *              and are depended on by many other classes.
  * @kind treemap
  * @id cpp/architecture/hub-classes

diff --git a/cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.qhelp b/cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.qhelp
@@ -5,44 +5,30 @@
 
 
 <overview>
-<p>This query shows the distribution of inheritance depth across all types, i.e. classes. Library types are ignored.</p>
+<p>This query shows the distribution of inheritance depth across all types, that is, classes. Library types are ignored.</p>
 
 <p>The result of this query is a line graph showing, for each number <i>n</i>, how many types have an inheritance depth of <i>n</i>, where
 the inheritance depth of a type is the length of a longest path in the inheritance hierarchy from top class to the type.</p>
-
-<p>When hovering the mouse pointer over a specific depth value, the number of types having this inheritance depth is displayed.</p>
-
 </overview>
-<section title="How to Address the Query Results">
+
+<recommendation>
 <p>The depth of a type is an indication of how deeply nested a type is in a given design. 
 Very deep types can be an indication of over-engineering, whereas a system with predominantly shallow types 
 may not be exploiting object-orientation to the full.</p>
+</recommendation>
 
-
-
-
-
-</section>
 <references>
 <li>
-Shyam R. Chidamber and Chris F. Kemerer.
-<a href="http://www.pitt.edu/~ckemerer/CK%20research%20papers/MetricForOOD_ChidamberKemerer94.pdf">A Metrics Suite for Object Oriented Design
-</a>.
+Shyam R. Chidamber and Chris F. Kemerer,
+<i><a href="http://www.pitt.edu/~ckemerer/CK%20research%20papers/MetricForOOD_ChidamberKemerer94.pdf">A Metrics Suite for Object Oriented Design
+</a></i>.
 IEEE Transactions on Software Engineering,
-20(6), pages 476-493, June 1994.
-
+20(6), pages 476-493, June 1994.</li>
 
-
-<a href="http://www.dmst.aueb.gr/dds/index.en.html">Diomides D. Spinnelis</a>.
-<a href="http://www.spinellis.gr/codequality/">Code Quality: The Open Source Perspective</a>.
-Addison-Wesley 2007.
-
-
-
-<a href="http://www.dmst.aueb.gr/dds/index.en.html">Diomides D. Spinnelis</a>.
-<a href="http://www.spinellis.gr/sw/ckjm/">ckjm - Chidamber and Kemerer Java Metrics</a>.
-(implementation of CK metrics), 2006.
-
-
-
-</li></references></qhelp>
+<li>
+Lutz Prechelt, Barbara Unger, Michael Philippsen, and Walter Tich, <i><a href="http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.159.2229&amp;rep=rep1&amp;type=pdf">A Controlled Experiment on Inheritance Depth as a Cost Factor for Code Maintenance
+</a></i>.
+Journal of Systems and Software, 65 (2):115-126, 2003.
+</li>
+</references>
+</qhelp>
diff --git a/cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.ql b/cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.ql
@@ -1,6 +1,6 @@
 /**
  * @name Inheritance depth distribution
- * @description Shows distribution of inheritance depth across all classes.
+ * @description Shows the distribution of inheritance depth across all classes.
  * @kind chart
  * @id cpp/architecture/inheritance-depth-distribution
  * @chart.type line

diff --git a/cpp/ql/src/Architecture/General Namespace-Level Information/CyclicNamespaces.qhelp b/cpp/ql/src/Architecture/General Namespace-Level Information/CyclicNamespaces.qhelp
@@ -7,19 +7,15 @@
 <overview>
 <p>This query shows namespaces that cyclically depend 
 on one another.</p>
-
-<p />
-
 </overview>
-<section title="How to Address the Query Results">
-<p>If there are cyclic dependencies between packages, they cannot be developed and tested independently. It is thus preferable to
-eliminate such cycles from the program.</p>
-
-
-
 
+<recommendation>
+<p>If there are cyclic dependencies between packages, they cannot be developed and tested independently. 
+It is better to eliminate such cycles from the program.</p>
+</recommendation>
 
-</section>
 <references>
 <li>Robert Martin's <a href="https://drive.google.com/file/d/0BwhCYaYDn8EgOGM2ZGFhNmYtNmE4ZS00OGY5LWFkZTYtMjE0ZGNjODQ0MjEx/view">Acyclic Dependencies Principle</a>.
-</li></references></qhelp>
+</li>
+</references>
+</qhelp>
diff --git a/cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.qhelp b/cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.qhelp
@@ -5,18 +5,15 @@
 
 
 <overview>
-<p>This query finds classes that belong to no namespace</p>
-
+<p>This query finds classes that belong to no namespace.</p>
 </overview>
-<section title="How to Address the Query Results">
-<p>If there are too many classes that belong to no namespace, consider creating namespaces to get a better project structure.</p>
-
-
 
+<recommendation>
+<p>If there are many classes that belong to no namespace, consider defining namespaces to create a better project structure.</p>
+</recommendation>
 
-
-</section>
 <references>
-
+<li>C++ reference: <a href="https://en.cppreference.com/w/cpp/language/namespace">Namespaces</a>
+ </li>
 </references>
 </qhelp>
diff --git a/cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.ql b/cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.ql
@@ -1,6 +1,6 @@
 /**
  * @name Global namespace classes
- * @description Finds classes that belong to no namespace
+ * @description Finds classes that belong to no namespace.
  * @kind table
  * @id cpp/architecture/global-namespace-classes
  */

diff --git a/cpp/ql/src/Architecture/General Namespace-Level Information/NamespaceDependencies.qhelp b/cpp/ql/src/Architecture/General Namespace-Level Information/NamespaceDependencies.qhelp
diff --git a/cpp/ql/src/Architecture/General Namespace-Level Information/NamespaceDependencies.ql b/cpp/ql/src/Architecture/General Namespace-Level Information/NamespaceDependencies.ql
@@ -1,6 +1,6 @@
 /**
  * @name Namespace dependencies
- * @description Shows dependencies between namespaces.
+ * @description Shows dependencies between namespaces as a hierarchical graph.
  * @kind graph
  * @id cpp/architecture/namespace-dependencies
  * @graph.layout hierarchical

diff --git a/cpp/ql/src/Best Practices/RuleOfThree.qhelp b/cpp/ql/src/Best Practices/RuleOfThree.qhelp
@@ -8,17 +8,13 @@
 <p>This query finds classes that define a destructor, a copy constructor, or a copy assignment operator, but not all three of them. The compiler generates default implementations for these functions, and since they deal with similar concerns it is likely that if the default implementation of one of them is not satisfactory, then neither are those of the others.</p>
 
 <p>The query flags any such class with a warning, and also display the list of generated warnings in the result view.</p>
-
 </overview>
-<section title="How to Address the Query Results">
-<p>Explicitly define the missing functions.</p>
-
-
-
 
+<recommendation>
+<p>Explicitly define the missing functions.</p>
+</recommendation>
 
-</section>
 <references>
-<li><a href="http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)">Wikipedia article</a>
-
-</li></references></qhelp>
+<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)">Rule of three (C++ programming)</a></li>
+</references>
+</qhelp>
diff --git a/cpp/ql/src/Critical/DeadCodeCondition.qhelp b/cpp/ql/src/Critical/DeadCodeCondition.qhelp
@@ -5,23 +5,26 @@
 
 
 <overview>
-<p>This rule finds branching statements with conditions that always evaluate to the same value. 
-More likely than not these conditions indicate a defect in the branching condition or are an artifact left behind after debugging.</p>
+<p>This query finds branching statements with conditions that always evaluate to the same value. 
+It is likely that these conditions indicate an error in the branching condition. 
+Alternatively, the conditions may have been left behind after debugging.</p>
 
 <include src="aliasAnalysisWarning.qhelp" />
-
 </overview>
-<recommendation>
-<p>Check the branch condition for defects, and verify that it isn't a remnant from debugging.</p>
 
+<recommendation>
+<p>Check the branch condition for logic errors. Check whether it is still required.</p>
 </recommendation>
-<example><sample src="DeadCodeCondition.cpp" />
-
-
-
-
-
 
+<example>
+<p>This example shows two branch conditions that always evaluate to the same value. 
+The two conditions and their associated branches should be deleted. 
+This will simplify the code and make it easier to maintain.</p>
 
+<sample src="DeadCodeCondition.cpp" />
 </example>
+
+<references>
+<li>SEI CERT C++ Coding Standard <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.</li>
+</references>
 </qhelp>
diff --git a/cpp/ql/src/Critical/DeadCodeFunction.cpp b/cpp/ql/src/Critical/DeadCodeFunction.cpp
@@ -2,7 +2,7 @@ class C {
 public:
 	void g() {
 		...
-		//f() was previously used but is now commented, orphaning f()
+		//f() was previously used but is now commented-out, orphaning f()
 		//f();
 		...
 	}

diff --git a/cpp/ql/src/Critical/DeadCodeFunction.qhelp b/cpp/ql/src/Critical/DeadCodeFunction.qhelp
@@ -3,28 +3,31 @@
   "qhelp.dtd">
 <qhelp>
 
-
 <overview>
-<p>This rule finds functions that are non-public, non-virtual and are never called. Dead functions are often deprecated pieces of code, and should be removed 
-as they may increase object code size, decrease code comprehensibility, and create the possibility of misuse.</p>
+<p>This query highlights functions that are non-public, non-virtual, and are never called. 
+Dead functions are often deprecated pieces of code, and should be removed. 
+If left in the code base they increase object code size, decrease code comprehensibility, and create the possibility of misuse.</p>
 
 <p>
-<code>public</code> and <code>protected</code> functions are not considered by the check, as they could be part of the program's
-API and could be used by external programs.
+<code>public</code> and <code>protected</code> functions are ignored by this query. 
+This type of function may be part of the program's API and could be used by external programs.
 </p>
 
 <include src="callGraphWarning.qhelp" />
-
 </overview>
-<recommendation>
-<p>Consider removing the function.</p>
 
+<recommendation>
+<p>Verify that the function is genuinely unused and consider removing it.</p>
 </recommendation>
-<example><sample src="DeadCodeFunction.cpp" />
-
 
 
+<example>
+<p>The example below includes a function <code>f</code> that is no longer used and should be deleted.</p>
+<sample src="DeadCodeFunction.cpp" />
+</example>
 
+<references>
+<li>SEI CERT C++ Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.</li>
+</references>
 
-</example>
 </qhelp>
diff --git a/cpp/ql/src/Critical/DeadCodeFunction.ql b/cpp/ql/src/Critical/DeadCodeFunction.ql
@@ -1,6 +1,6 @@
 /**
  * @name Function is never called
- * @description A function is never called, and should be considered for removal. Unused functions may increase object size, decrease readability and create the possibility of misuse.
+ * @description Unused functions may increase object size, decrease readability, and create the possibility of misuse.
  * @kind problem
  * @id cpp/dead-code-function
  * @problem.severity warning
-Original file line number
+Diff line change
@@ Expand Up / @@ -2,7 +2,7 @@ class C { @@
     public:
     	void g() {
     		...
-    		//f() was previously used but is now commented, orphaning f()
+    		//f() was previously used but is now commented-out, orphaning f()
     		//f();
     		...
     	}
@@ Expand Down @@