Merge branch 'master' into hresult-boolean-qhelp

Author: jbj

.gitignore

@@ -8,8 +8,7 @@
 # qltest projects and artifacts
 */ql/test/**/*.testproj
 */ql/test/**/*.actual
-/.vs/slnx.sqlite
-/.vs/ql/v15/Browse.VC.opendb
-/.vs/ql/v15/Browse.VC.db
-/.vs/ProjectSettings.json
 
+# Visual studio temporaries, except a file used by QL4VS
+.vs/*
+!.vs/VSWorkspaceSettings.json

change-notes/1.19/analysis-cpp.md

@@ -8,13 +8,17 @@
 |-----------------------------|-----------|--------------------------------------------------------------------|
 | Cast between HRESULT and a Boolean type (`cpp/hresult-boolean-conversion`) | external/cwe/cwe-253 | Finds logic errors caused by mistakenly treating the Windows `HRESULT` type as a Boolean instead of testing it with the appropriate macros. Enabled by default. |
 | Setting a DACL to `NULL` in a `SECURITY_DESCRIPTOR` (`cpp/unsafe-dacl-security-descriptor`) | external/cwe/cwe-732 | This query finds code that creates world-writable objects on Windows by setting their DACL to `NULL`. Enabled by default. |
+| Cast from char* to wchar_t* | security, external/cwe/cwe-704 | Detects potentially dangerous casts from char* to wchar_t*.  Enabled by default on LGTM. |
 
 ## Changes to existing queries
 
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. |
-
+| 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. |
+| Call to memory access function may overflow buffer | More correct results | Array indexing with a negative index is now detected by this query. |
+| Suspicious add with sizeof | Fewer false positive results | Arithmetic with void pointers (where allowed) is now excluded from this query. |
+| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed.  Expected argument types are determined more accurately, especially for wide string and pointer types.  Custom (non-standard) formatting functions are also identified more accurately. |
 
 ## Changes to QL libraries
 

change-notes/1.19/analysis-javascript.md

@@ -9,22 +9,30 @@
 * 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)
 
+* The type inference now handles nested imports (that is, imports not appearing at the toplevel). This may yield fewer false-positive results on projects that use this non-standard language feature.
 
 ## New queries
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |
 |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | Enabling Node.js integration for Electron web content renderers (`js/enabling-electron-renderer-node-integration`) | security, frameworks/electron, external/cwe/cwe-094  | Highlights Electron web content renderer preferences with Node.js integration enabled, indicating a violation of [CWE-94](https://cwe.mitre.org/data/definitions/94.html). Results are not shown on LGTM by default. |
-| Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
+| Host header poisoning in email generation |  security, external/cwe/cwe-640 | Highlights code that generates emails with links that can be hijacked by HTTP host header poisoning, indicating a violation of [CWE-640](https://cwe.mitre.org/data/definitions/640.html). Results shown on LGTM by default.  |
 | Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
+| Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
+| Unclear precedence of nested operators (`js/unclear-operator-precedence`) | maintainability, correctness, external/cwe/cwe-783 | Highlights nested binary operators whose relative precedence is easy to misunderstand. Results shown on LGTM by default. |
 
 ## Changes to existing queries
 
 | **Query**                      | **Expected impact**        | **Change**                                   |
 |--------------------------------|----------------------------|----------------------------------------------|
+| Useless assignment to local variable | Fewer false-positive results | This rule now recognizes additional ways default values can be set. |
 | Regular expression injection | Fewer false-positive results | This rule now identifies calls to `String.prototype.search` with more precision. |
 | Unbound event handler receiver | Fewer false-positive results | This rule now recognizes additional ways class methods can be bound. |
 | Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |
 | Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
+| Server-side URL redirect | More results | This rule now recognizes redirection calls in more cases. |
+| Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
 
 ## Changes to QL libraries
+
+* The flow configuration framework now supports distinguishing and tracking different kinds of taint, specified by an extensible class `FlowLabel` (which can also be referred to by its alias `TaintKind`).

cpp/ql/src/Documentation/CommentedOutCode.qll

@@ -119,7 +119,7 @@ class CommentBlock extends Comment {
      */
     predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
         this.getLocation().hasLocationInfo(filepath, startline, startcolumn, _, _) and
-        this.lastComment().getLocation().hasLocationInfo(filepath, _, _, endline, endcolumn)
+        this.lastComment().getLocation().hasLocationInfo(_, _, _, endline, endcolumn)
     }
 }
 

cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql

@@ -22,13 +22,15 @@ import UnsignedGEZero
 // #define PRINTMSG(val,msg) { if (val >= PRINTLEVEL) printf(msg); }
 //
 // So to reduce the number of false positives, we do not report a result if
-// the comparison is in a macro expansion.
+// the comparison is in a macro expansion. Similarly for template
+// instantiations.
 from
   ComparisonOperation cmp, SmallSide ss,
   float left, float right, boolean value,
   string reason
 where
   not cmp.isInMacroExpansion() and
+  not cmp.isFromTemplateInstantiation(_) and
   reachablePointlessComparison(cmp, left, right, value, ss) and
 
   // a comparison between an enum and zero is always valid because whether

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

@@ -25,7 +25,7 @@ predicate stringType(Type t, Type charType) {
       charType = t.(ArrayType).getBaseType()
     ) and (
       charType.getUnspecifiedType() instanceof CharType or
-      charType.getUnspecifiedType() instanceof WideCharType
+      charType.getUnspecifiedType() instanceof Wchar_t
     )
   )
   or

cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql

@@ -25,7 +25,8 @@ private predicate formattingFunctionCallExpectedType(FormattingFunctionCall ffc,
       ffc.getTarget() = f and
       f.getFormatParameterIndex() = i and
       ffc.getArgument(i) = fl and
-      fl.getConversionType(pos) = expected
+      fl.getConversionType(pos) = expected and
+      count(fl.getConversionType(pos)) = 1
     )
 }
 
@@ -66,30 +67,15 @@ predicate formatOtherArgType(FormattingFunctionCall ffc, int pos, Type expected,
 class ExpectedType extends Type
 {
   ExpectedType() {
-    formatArgType(_, _, this, _, _) or
-    formatOtherArgType(_, _, this, _, _) or
-    exists(ExpectedType t |
-      this = t.(PointerType).getBaseType()
+  	exists(Type t |
+      (
+        formatArgType(_, _, t, _, _) or
+        formatOtherArgType(_, _, t, _, _)
+      ) and this = t.getUnspecifiedType()
     )
   }
 }
 
-/**
- * Gets an 'interesting' type that can be reached from `t` by removing
- * typedefs and specifiers.  Note that this does not always mean removing
- * all typedefs and specifiers as `Type.getUnspecifiedType()` would, for
- * example if the interesting type is itself a typedef.
- */
-ExpectedType getAnUnderlyingExpectedType(Type t) {
-  (
-    result = t
-  ) or (
-    result = getAnUnderlyingExpectedType(t.(TypedefType).getBaseType())
-  ) or (
-    result = getAnUnderlyingExpectedType(t.(SpecifiedType).getBaseType())
-  )
-}
-
 /**
  * Holds if it is safe to display a value of type `actual` when `printf`
  * expects a value of type `expected`.
@@ -100,59 +86,48 @@ ExpectedType getAnUnderlyingExpectedType(Type t) {
  * are converted to `double`.
  */
 predicate trivialConversion(ExpectedType expected, Type actual) {
-  formatArgType(_, _, expected, _, actual) and
-
-  exists(Type actualU |
-    actualU = actual.getUnspecifiedType() and
+  exists(Type exp, Type act |
+    formatArgType(_, _, exp, _, act) and
+    expected = exp.getUnspecifiedType() and
+    actual = act.getUnspecifiedType()
+  ) and (
     (
-      (
-        // allow a pointer type to be displayed with `%p`
-        expected instanceof VoidPointerType and actualU instanceof PointerType
-      ) or (
-        // allow a function pointer type to be displayed with `%p`
-        expected instanceof VoidPointerType and actualU instanceof FunctionPointerType and expected.getSize() = actual.getSize()
-      ) or (
-        // allow an `enum` type to be displayed with `%i`, `%c` etc
-        expected instanceof IntegralType and actualU instanceof Enum
-      ) or (
-        // allow any `char *` type to be displayed with `%s`
-        expected instanceof CharPointerType and actualU instanceof CharPointerType
-      ) or (
-        // allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
-        // with `%ws`
-        expected.(PointerType).getBaseType().hasName("wchar_t") and
-        exists(Wchar_t t |
-          actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
-        )
-      ) or (
-        // allow an `int` (or anything promoted to `int`) to be displayed with `%c`
-        expected instanceof CharType and actualU instanceof IntType
-      ) or (
-        // allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
-        expected instanceof Wchar_t and actualU instanceof IntType
-      ) or (
-        expected instanceof UnsignedCharType and actualU instanceof IntType
-      ) or (
-        // allow the underlying type of a `size_t` (e.g. `unsigned long`) for
-        // `%zu`, since this is the type of a `sizeof` expression
-        expected instanceof Size_t and
-        actual.getUnspecifiedType() = expected.getUnspecifiedType()
-      ) or (
-        // allow the underlying type of a `ssize_t` (e.g. `long`) for `%zd`
-        expected instanceof Ssize_t and
-        actual.getUnspecifiedType() = expected.getUnspecifiedType()
-      ) or (
-        // allow any integral type of the same size 
-        // (this permits signedness changes)
-        expected.(IntegralType).getSize() = actualU.(IntegralType).getSize()
-      ) or (
-        // allow a pointer to any integral type of the same size
-        // (this permits signedness changes)
-        expected.(PointerType).getBaseType().(IntegralType).getSize() = actualU.(PointerType).getBaseType().(IntegralType).getSize()
-      ) or (
-        // allow expected, or a typedef or specified version of expected
-        expected = getAnUnderlyingExpectedType(actual)
+      // allow a pointer type to be displayed with `%p`
+      expected instanceof VoidPointerType and actual instanceof PointerType
+    ) or (
+      // allow a function pointer type to be displayed with `%p`
+      expected instanceof VoidPointerType and actual instanceof FunctionPointerType and expected.getSize() = actual.getSize()
+    ) or (
+      // allow an `enum` type to be displayed with `%i`, `%c` etc
+      expected instanceof IntegralType and actual instanceof Enum
+    ) or (
+      // allow any `char *` type to be displayed with `%s`
+      expected instanceof CharPointerType and actual instanceof CharPointerType
+    ) or (
+      // allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
+      // with `%ws`
+      expected.(PointerType).getBaseType().hasName("wchar_t") and
+      exists(Wchar_t t |
+        actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
       )
+    ) or (
+      // allow an `int` (or anything promoted to `int`) to be displayed with `%c`
+      expected instanceof CharType and actual instanceof IntType
+    ) or (
+      // allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
+      expected instanceof Wchar_t and actual instanceof IntType
+    ) or (
+      expected instanceof UnsignedCharType and actual instanceof IntType
+    ) or (
+      // allow any integral type of the same size 
+      // (this permits signedness changes)
+      expected.(IntegralType).getSize() = actual.(IntegralType).getSize()
+    ) or (
+      // allow a pointer to any integral type of the same size
+      // (this permits signedness changes)
+      expected.(PointerType).getBaseType().(IntegralType).getSize() = actual.(PointerType).getBaseType().(IntegralType).getSize()
+    ) or (
+      expected = actual
     )
   )
 }
@@ -164,16 +139,16 @@ int sizeof_IntType() {
   exists(IntType it | result = it.getSize())
 }
 
-from FormattingFunctionCall ffc, int n, Expr arg, ExpectedType expected, Type actual
+from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual
 where (
         (
           formatArgType(ffc, n, expected, arg, actual) and
-          not trivialConversion(expected, actual)
+          not trivialConversion(expected.getUnspecifiedType(), actual.getUnspecifiedType())
         )
         or
         (
           formatOtherArgType(ffc, n, expected, arg, actual) and
-          not actual.getUnderlyingType().(IntegralType).getSize() = sizeof_IntType()
+          not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
         )
       )
       and not arg.isAffectedByMacro()

cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToMemset.ql

@@ -41,11 +41,13 @@ Type stripType(Type t) {
   result = stripType(t.(ArrayType).getBaseType()) or
   result = stripType(t.(ReferenceType).getBaseType()) or
   result = stripType(t.(SpecifiedType).getBaseType()) or
+  result = stripType(t.(Decltype).getBaseType()) or
   (
     not t instanceof TypedefType and
     not t instanceof ArrayType and
     not t instanceof ReferenceType and
     not t instanceof SpecifiedType and
+    not t instanceof Decltype and
     result = t
   )
 }

cpp/ql/src/Likely Bugs/UseInOwnInitializer.ql

@@ -11,10 +11,21 @@
 
 import cpp
 
-from Initializer init, Variable v, VariableAccess va
-where init.getDeclaration() = v
-  and va.getTarget() = v
-  and va.getParent*() = init
+class VariableAccessInInitializer extends VariableAccess {
+  Variable var;
+  Initializer init;
+  VariableAccessInInitializer() {
+    init.getDeclaration() = var and
+    init.getExpr().getAChild*() = this
+  }
+
+  predicate initializesItself(Variable v, Initializer i) {
+    v = var and i = init and var = this.getTarget()
+  }
+}
+
+from Initializer init, Variable v, VariableAccessInInitializer va
+where va.initializesItself(v, init)
   and (
     va.hasLValueToRValueConversion() or
     exists (Assignment assn | assn.getLValue() = va) or

cpp/ql/src/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Semmle C/C++ Default Queries
 Bundle-SymbolicName: com.semmle.plugin.semmlecode.cpp.queries;singleton:=true
-Bundle-Version: 1.18.0.qualifier
+Bundle-Version: 1.18.1.qualifier
 Bundle-Vendor: Semmle Ltd.
 Bundle-ActivationPolicy: lazy
-Require-Bundle: com.semmle.plugin.qdt.ui;bundle-version="[1.18.0.qualifier,1.18.0.qualifier]"
+Require-Bundle: com.semmle.plugin.qdt.ui;bundle-version="[1.18.1.qualifier,1.18.1.qualifier]"