Merge branch 'master' into rdmarsh/cpp/ir-flow-through-outparams

Author: Robert Marsh

change-notes/1.24/analysis-cpp.md

@@ -26,6 +26,10 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
 
 ## Changes to libraries
 
+* The data-flow library has been improved when flow through functions needs to be
+  combined with both taint tracking and flow through fields allowing more flow
+  to be tracked. This affects and improves some security queries, which may
+  report additional results.
 * Created the `semmle.code.cpp.models.interfaces.Allocation` library to model allocation such as `new` expressions and calls to `malloc`. This in intended to replace the functionality in `semmle.code.cpp.commons.Alloc` with a more consistent and useful interface.
 * Created the `semmle.code.cpp.models.interfaces.Deallocation` library to model deallocation such as `delete` expressions and calls to `free`. This in intended to replace the functionality in `semmle.code.cpp.commons.Alloc` with a more consistent and useful interface.
 * The new class `StackVariable` should be used in place of `LocalScopeVariable`

change-notes/1.24/analysis-csharp.md

@@ -29,6 +29,10 @@ The following changes in version 1.24 affect C# analysis in all applications.
 
 ## Changes to libraries
 
+* The data-flow library has been improved when flow through methods needs to be
+  combined with both taint tracking and flow through fields allowing more flow
+  to be tracked. This affects and improves most security queries, which may
+  report additional results.
 * The taint tracking library now tracks flow through (implicit or explicit) conversion operator calls.
 * [Code contracts](https://docs.microsoft.com/en-us/dotnet/framework/debug-trace-profile/code-contracts) are now recognized, and are treated like any other assertion methods.
 * Expression nullability flow state is given by the predicates `Expr.hasNotNullFlowState()` and `Expr.hasMaybeNullFlowState()`.

change-notes/1.24/analysis-java.md

@@ -25,10 +25,17 @@ The following changes in version 1.24 affect Java analysis in all applications.
 
 ## Changes to libraries
 
+* The data-flow library has been improved when flow through methods needs to be
+  combined with both taint tracking and flow through fields allowing more flow
+  to be tracked. This affects and improves most security queries, which may
+  report additional results.
 * Identification of test classes has been improved. Previously, one of the
   match conditions would classify any class with a name containing the string
   "Test" as a test class, but now this matching has been replaced with one that
   looks for the occurrence of actual unit-test annotations. This affects the
   general file classification mechanism and thus suppression of alerts, and
   also any security queries using taint tracking, as test classes act as
   default barriers stopping taint flow.
+* Parentheses are now no longer modelled directly in the AST, that is, the
+  `ParExpr` class is empty. Instead, a parenthesized expression can be
+  identified with the `Expr.isParenthesized()` member predicate.

change-notes/1.24/analysis-javascript.md

@@ -7,7 +7,9 @@
 * Imports with the `.js` extension can now be resolved to a TypeScript file,
   when the import refers to a file generated by TypeScript.
 
-- The analysis of sanitizer guards has improved, leading to fewer false-positive results from the security queries.
+* Imports that rely on path-mappings from a `tsconfig.json` file can now be resolved.
+
+* The analysis of sanitizer guards has improved, leading to fewer false-positive results from the security queries.
 
 * Support for the following frameworks and libraries has been improved:
   - [react](https://www.npmjs.com/package/react)
@@ -18,6 +20,7 @@
   - [Socket.IO](https://socket.io/)
   - [ws](https://github.com/websockets/ws)
   - [WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API)
+  - [Koa](https://www.npmjs.com/package/koa)
 
 ## New queries
 

cpp/ql/src/Architecture/FeatureEnvy.ql

@@ -25,7 +25,8 @@ predicate functionUsesFunction(Function source, Function f, File target) {
 }
 
 predicate dependencyCount(Function source, File target, int res) {
-  res = strictcount(Declaration d |
+  res =
+    strictcount(Declaration d |
       functionUsesVariable(source, d, target) or
       functionUsesFunction(source, d, target)
     )

cpp/ql/src/Architecture/General Top-Level Information/GeneralStatistics.ql

@@ -38,14 +38,16 @@ where
   n = count(Function f | f.fromSource()).toString()
   or
   l = "Number of Lines Of Code" and
-  n = sum(File f, int toSum |
+  n =
+    sum(File f, int toSum |
       f.fromSource() and toSum = f.getMetrics().getNumberOfLinesOfCode()
     |
       toSum
     ).toString()
   or
   l = "Self-Containedness" and
-  n = (
+  n =
+    (
         100 * sum(Class c | c.fromSource() | c.getMetrics().getEfferentSourceCoupling()) /
           sum(Class c | c.fromSource() | c.getMetrics().getEfferentCoupling())
       ).toString() + "%"

cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyFields.ql

@@ -80,11 +80,8 @@ class VariableDeclarationLine extends TVariableDeclarationInfo {
    * (that is, the first is 0, the second is 1 and so on).
    */
   private int getRank() {
-    line = rank[result](VariableDeclarationLine vdl, int l |
-        vdl = TVariableDeclarationLine(c, f, l)
-      |
-        l
-      )
+    line =
+      rank[result](VariableDeclarationLine vdl, int l | vdl = TVariableDeclarationLine(c, f, l) | l)
   }
 
   /**
@@ -133,7 +130,8 @@ class VariableDeclarationGroup extends VariableDeclarationLine {
    * Gets the number of uniquely named `VariableDeclarationEntry`s in this group.
    */
   int getCount() {
-    result = count(VariableDeclarationLine l |
+    result =
+      count(VariableDeclarationLine l |
         l = getProximateNext*()
       |
         l.getAVDE().getVariable().getName()
@@ -166,7 +164,8 @@ class ExtClass extends Class {
 
 from ExtClass c, int n, VariableDeclarationGroup vdg, string suffix
 where
-  n = strictcount(string fieldName |
+  n =
+    strictcount(string fieldName |
       exists(Field f |
         f.getDeclaringType() = c and
         fieldName = f.getName() and

cpp/ql/src/Best Practices/Likely Errors/EmptyBlock.ql

@@ -50,21 +50,24 @@ class BlockOrNonChild extends Element {
 
   private int getNonContiguousStartRankIn(AffectedFile file) {
     // When using `rank` with `order by`, the ranks may not be contiguous.
-    this = rank[result](BlockOrNonChild boc, int startLine, int startCol |
+    this =
+      rank[result](BlockOrNonChild boc, int startLine, int startCol |
         boc.getLocation().hasLocationInfo(file.getAbsolutePath(), startLine, startCol, _, _)
       |
         boc order by startLine, startCol
       )
   }
 
   int getStartRankIn(AffectedFile file) {
-    this.getNonContiguousStartRankIn(file) = rank[result](int rnk |
+    this.getNonContiguousStartRankIn(file) =
+      rank[result](int rnk |
         exists(BlockOrNonChild boc | boc.getNonContiguousStartRankIn(file) = rnk)
       )
   }
 
   int getNonContiguousEndRankIn(AffectedFile file) {
-    this = rank[result](BlockOrNonChild boc, int endLine, int endCol |
+    this =
+      rank[result](BlockOrNonChild boc, int endLine, int endCol |
         boc.getLocation().hasLocationInfo(file.getAbsolutePath(), _, _, endLine, endCol)
       |
         boc order by endLine, endCol
@@ -79,9 +82,8 @@ predicate emptyBlockContainsNonchild(Block b) {
   emptyBlock(_, b) and
   exists(BlockOrNonChild c, AffectedFile file |
     c.(BlockOrNonChild).getStartRankIn(file) = 1 + b.(BlockOrNonChild).getStartRankIn(file) and
-    c.(BlockOrNonChild).getNonContiguousEndRankIn(file) < b
-          .(BlockOrNonChild)
-          .getNonContiguousEndRankIn(file)
+    c.(BlockOrNonChild).getNonContiguousEndRankIn(file) <
+      b.(BlockOrNonChild).getNonContiguousEndRankIn(file)
   )
 }
 

cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll

@@ -307,7 +307,8 @@ predicate nonTrivialValue(string value, Literal literal) {
 }
 
 predicate valueOccurrenceCount(string value, int n) {
-  n = strictcount(Location loc |
+  n =
+    strictcount(Location loc |
       exists(Literal lit | lit.getLocation() = loc | nonTrivialValue(value, lit)) and
       // Exclude generated files (they do not have the same maintainability
       // concerns as ordinary source files)
@@ -338,7 +339,8 @@ predicate check(Literal lit, string value, int n, File f) {
 }
 
 predicate checkWithFileCount(string value, int overallCount, int fileCount, File f) {
-  fileCount = strictcount(Location loc |
+  fileCount =
+    strictcount(Location loc |
       exists(Literal lit | lit.getLocation() = loc | check(lit, value, overallCount, f))
     )
 }
@@ -364,7 +366,8 @@ predicate firstOccurrence(Literal lit, string value, int n) {
 predicate magicConstant(Literal e, string msg) {
   exists(string value, int n |
     firstOccurrence(e, value, n) and
-    msg = "Magic constant: literal '" + value + "' is repeated " + n.toString() +
+    msg =
+      "Magic constant: literal '" + value + "' is repeated " + n.toString() +
         " times and should be encapsulated in a constant."
   )
 }

cpp/ql/src/Best Practices/RuleOfTwo.ql

@@ -28,13 +28,15 @@ import cpp
 // design question and carries has no safety risk.
 predicate generatedCopyAssignment(CopyConstructor cc, string msg) {
   cc.getDeclaringType().hasImplicitCopyAssignmentOperator() and
-  msg = "No matching copy assignment operator in class " + cc.getDeclaringType().getName() +
+  msg =
+    "No matching copy assignment operator in class " + cc.getDeclaringType().getName() +
       ". It is good practice to match a copy constructor with a " + "copy assignment operator."
 }
 
 predicate generatedCopyConstructor(CopyAssignmentOperator ca, string msg) {
   ca.getDeclaringType().hasImplicitCopyConstructor() and
-  msg = "No matching copy constructor in class " + ca.getDeclaringType().getName() +
+  msg =
+    "No matching copy constructor in class " + ca.getDeclaringType().getName() +
       ". It is good practice to match a copy assignment operator with a " + "copy constructor."
 }