Merge operation

Author: raulgarciamsft

.gitignore

@@ -8,10 +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
 
-/.vs/ql/v15/.suo
-/.vs/VSWorkspaceState.json
+# Visual studio temporaries, except a file used by QL4VS
+.vs/*
+!.vs/VSWorkspaceSettings.json

change-notes/1.19/analysis-javascript.md

@@ -4,6 +4,8 @@
 
 * Modelling of taint flow through array operations has been improved. This may give additional results for the security queries.
 
+* The taint tracking library now recognizes additional sanitization patterns. This may give fewer false-positive results for the security queries.
+
 * 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)
 
@@ -14,6 +16,7 @@
 |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | 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. |
+| 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. |
 
 ## Changes to existing queries
 

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/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]"

cpp/ql/src/Security/CWE/CWE-468/SuspiciousAddWithSizeof.ql

@@ -13,18 +13,19 @@
 import cpp
 import IncorrectPointerScalingCommon
 
-private predicate isCharPtrExpr(Expr e) {
+private predicate isCharSzPtrExpr(Expr e) {
   exists (PointerType pt
   | pt = e.getFullyConverted().getUnderlyingType()
-  | pt.getBaseType().getUnspecifiedType() instanceof CharType)
+  | pt.getBaseType().getUnspecifiedType() instanceof CharType
+  or pt.getBaseType().getUnspecifiedType() instanceof VoidType)
 }
 
 from Expr sizeofExpr, Expr e
 where
   // If we see an addWithSizeof then we expect the type of
-  // the pointer expression to be char*. Otherwise it is probably
-  // a mistake.
-  addWithSizeof(e, sizeofExpr, _) and not isCharPtrExpr(e)
+  // the pointer expression to be char* or void*. Otherwise it
+  // is probably a mistake.
+  addWithSizeof(e, sizeofExpr, _) and not isCharSzPtrExpr(e)
 select
   sizeofExpr,
   "Suspicious sizeof offset in a pointer arithmetic expression. " +

cpp/ql/src/definitions.qll

@@ -102,7 +102,8 @@ private predicate constructorCallStartLoc(ConstructorCall cc, File f, int line,
 
 /**
  * Holds if `f`, `line`, `column` indicate the start character
- * of `tm`, which mentions `t`.
+ * of `tm`, which mentions `t`. Type mentions for instantiations
+ * are filtered out.
  */
 private predicate typeMentionStartLoc(TypeMention tm, Type t, File f, int line, int column) {
   exists(Location l |
@@ -111,7 +112,8 @@ private predicate typeMentionStartLoc(TypeMention tm, Type t, File f, int line,
     l.getStartLine() = line and
     l.getStartColumn() = column
   ) and
-  t = tm.getMentionedType()
+  t = tm.getMentionedType() and
+  not t instanceof ClassTemplateInstantiation
 }
 
 /**

cpp/ql/src/semmle/code/cpp/Element.qll

@@ -2,41 +2,38 @@ import semmle.code.cpp.Location
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
 
-/**
- * Get the `@element` that represents this `@element`.
- * Normally this will simply be `e`, but sometimes it is not.
- * For example, for an incomplete struct `e` the result may be a
- * complete struct with the same name.
- */
-private cached @element resolveElement(@element e) {
-  if isClass(e)
-  then result = resolveClass(e)
-  else result = e
-}
-
 /**
  * Get the `Element` that represents this `@element`.
  * Normally this will simply be a cast of `e`, but sometimes it is not.
  * For example, for an incomplete struct `e` the result may be a
  * complete struct with the same name.
  */
+pragma[inline]
 Element mkElement(@element e) {
-  result = resolveElement(e)
+  unresolveElement(result) = e
 }
 
 /**
- * Get an `@element` that resolves to the `Element`. This should
+ * INTERNAL: Do not use.
+ *
+ * Gets an `@element` that resolves to the `Element`. This should
  * normally only be called from member predicates, where `e` is not
  * `this` and you need the result for an argument to a database
  * extensional.
  * See `underlyingElement` for when `e` is `this`.
  */
+pragma[inline]
 @element unresolveElement(Element e) {
-  resolveElement(result) = e
+  not result instanceof @usertype and
+  result = e
+  or
+  e = resolveClass(result)
 }
 
 /**
- * Get the `@element` that this `Element` extends. This should normally
+ * INTERNAL: Do not use.
+ *
+ * Gets the `@element` that this `Element` extends. This should normally
  * only be called from member predicates, where `e` is `this` and you
  * need the result for an argument to a database extensional.
  * See `unresolveElement` for when `e` is not `this`.
@@ -53,10 +50,6 @@ Element mkElement(@element e) {
  * `getLocation`, or `hasLocationInfo`.
  */
 class ElementBase extends @element {
-  ElementBase() {
-    this = resolveElement(_)
-  }
-
   /** Gets a textual representation of this element. */
   string toString() { none() }
 }

cpp/ql/src/semmle/code/cpp/Specifier.qll

@@ -294,13 +294,13 @@ class AttributeArgument extends Element, @attribute_arg {
   }
 
   override string toString() {
-    if exists (@attribute_arg_empty self | mkElement(self) = this)
+    if exists (@attribute_arg_empty self | self = underlyingElement(this))
       then result = "empty argument"
       else exists (string prefix, string tail
            | (if exists(getName())
                 then prefix = getName() + "="
                 else prefix = "") and
-             (if exists (@attribute_arg_type self | mkElement(self) = this)
+             (if exists (@attribute_arg_type self | self = underlyingElement(this))
                 then tail = getValueType().getName()
                 else tail = getValueText()) and
              result = prefix + tail)

cpp/ql/src/semmle/code/cpp/Type.qll

@@ -7,6 +7,8 @@ private import semmle.code.cpp.internal.ResolveClass
  * A C/C++ type.
  */
 class Type extends Locatable, @type {
+  Type() { isType(underlyingElement(this)) }
+
   /**
    * Gets the name of this type.
    */