Master-to-next merge

.editorconfig

@@ -1,2 +1,2 @@
-[*.{ql,qll,qlref,dbscheme,qhelp}]
-end_of_line = lf
+[*]
+end_of_line = lf

.gitattributes

@@ -1,17 +1,48 @@
-# The following file types will be normalized to LF line endings in the Git
-# database, and will keep those LF line endings in the working tree even on
-# Windows. Any other files will have whatever line endings they had when they
-# were committed. If you add new entries below, you should renormalize the
-# affected files by running the following from the root of this repo (requires
-# Git 2.16 or greater):
+# Text files will be normalized to LF line endings in the Git database, and will keep those LF line
+# endings in the working tree even on Windows. If you make changes below, you should renormalize the
+# affected files by running the following from the root of this repo (requires Git 2.16 or greater):
 #
 # git add --renormalize .
 # git status  [just to show what files were renormalized]
 # git commit -m "Normalize line endings"
-#
-# Also, please update .editorconfig to handle any new entries as well.
-*.ql eol=lf
-*.qll eol=lf
-*.qlref eol=lf
-*.dbscheme eol=lf
-*.qhelp eol=lf
+
+# Anything Git auto-detects as text gets normalized and checked out as LF
+* text=auto eol=lf
+
+# Explicitly set a bunch of known extensions to text, in case auto detection gets confused.
+*.ql text
+*.qll text
+*.qlref text
+*.dbscheme text
+*.qhelp text
+*.html text
+*.htm text
+*.xhtml text
+*.xhtm text
+*.js text
+*.mjs text
+*.ts text
+*.json text
+*.yml text
+*.yaml text
+*.c text
+*.cpp text
+*.h text
+*.hpp text
+*.md text
+*.stats text
+*.xml text
+*.sh text
+*.pl text
+*.java text
+*.cs text
+*.py text
+*.lua text
+*.expected text
+
+# Explicitly set a bunch of known extensions to binary, because Git < 2.10 will treat
+# `* text=auto eol=lf` as `* text eol=lf`
+*.png -text
+*.jpg -text
+*.jpeg -text
+*.gif -text

.gitignore

@@ -8,3 +8,8 @@
 # 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
+

CODEOWNERS

@@ -1,2 +1,3 @@
 /csharp/ @Semmle/cs
+/java/ @Semmle/java
 /javascript/ @Semmle/js

change-notes/1.19/analysis-cpp.md

@@ -0,0 +1,20 @@
+# Improvements to C/C++ analysis
+
+## General improvements
+
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+| *@name of query (Query ID)* | *Tags*    |*Aim of the new query and whether it is enabled by default or not*  |
+
+## 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. |
+
+
+## Changes to QL libraries
+
+* Added a hash consing library for structural comparison of expressions.

change-notes/1.19/analysis-javascript.md

@@ -2,17 +2,27 @@
 
 ## General improvements
 
+* Modelling of taint flow through array operations has been improved. This may give additional 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)
+
+
 ## New queries
 
-| **Query**                   | **Tags**  | **Purpose**                                                        |
-|-----------------------------|-----------|--------------------------------------------------------------------|
-| *@name of query (Query ID)* | *Tags*    |*Aim of the new query and whether it is enabled by default or not*  |
+| **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. |
+| 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
 
 | **Query**                      | **Expected impact**        | **Change**                                   |
 |--------------------------------|----------------------------|----------------------------------------------|
 | 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. |
 
 ## Changes to QL libraries

config/identical-files.json

@@ -54,5 +54,10 @@
     "C++ SSA SSAConstruction": [
         "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll",
         "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll"
+    ],
+    "C++ IR ValueNumber": [
+        "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll",
+        "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll",
+        "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll"
     ]
 }

cpp/config/suites/security/cwe-120

@@ -1,13 +1,13 @@
-# CWE-120: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow')
-+ semmlecode-cpp-queries/Security/CWE/CWE-120/UnboundedWrite.ql: /CWE/CWE-120
-  @name Unbounded write (CWE-120)
-+ semmlecode-cpp-queries/Security/CWE/CWE-120/BadlyBoundedWrite.ql: /CWE/CWE-120
-  @name Badly bounded write (CWE-120)
-+ semmlecode-cpp-queries/Security/CWE/CWE-120/OverrunWrite.ql: /CWE/CWE-120
-  @name Potentially overrunning write (CWE-120)
-+ semmlecode-cpp-queries/Security/CWE/CWE-120/OverrunWriteFloat.ql: /CWE/CWE-120
-  @name Potentially overrunning write with float to string conversion (CWE-120)
-+ semmlecode-cpp-queries/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql: /CWE/CWE-120
-  @name Array offset used before range check (CWE-120)
-+ semmlecode-cpp-queries/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql: /CWE/CWE-120
-    @name Potentially unsafe use of strcat (CWE-120)
+# CWE-120: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow')
++ semmlecode-cpp-queries/Security/CWE/CWE-120/UnboundedWrite.ql: /CWE/CWE-120
+  @name Unbounded write (CWE-120)
++ semmlecode-cpp-queries/Security/CWE/CWE-120/BadlyBoundedWrite.ql: /CWE/CWE-120
+  @name Badly bounded write (CWE-120)
++ semmlecode-cpp-queries/Security/CWE/CWE-120/OverrunWrite.ql: /CWE/CWE-120
+  @name Potentially overrunning write (CWE-120)
++ semmlecode-cpp-queries/Security/CWE/CWE-120/OverrunWriteFloat.ql: /CWE/CWE-120
+  @name Potentially overrunning write with float to string conversion (CWE-120)
++ semmlecode-cpp-queries/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql: /CWE/CWE-120
+  @name Array offset used before range check (CWE-120)
++ semmlecode-cpp-queries/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql: /CWE/CWE-120
+    @name Potentially unsafe use of strcat (CWE-120)

cpp/config/suites/security/cwe-121

@@ -1,3 +1,3 @@
-# CWE-121: Stack-based Buffer Overflow
-+ semmlecode-cpp-queries/Security/CWE/CWE-121/UnterminatedVarargsCall.ql: /CWE/CWE-121
-    @name Unterminated variadic call (CWE-121)
+# CWE-121: Stack-based Buffer Overflow
++ semmlecode-cpp-queries/Security/CWE/CWE-121/UnterminatedVarargsCall.ql: /CWE/CWE-121
+    @name Unterminated variadic call (CWE-121)

cpp/config/suites/security/cwe-131

@@ -1,7 +1,7 @@
-# CWE-131: Incorrect Calculation of Buffer Size
-+ semmlecode-cpp-queries/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql: /CWE/CWE-131
-    @name No space for zero terminator (CWE-131)
-+ semmlecode-cpp-queries/Critical/SizeCheck.ql: /CWE/CWE-131
-    @name Not enough memory allocated for pointer type (CWE-131)
-+ semmlecode-cpp-queries/Critical/SizeCheck2.ql: /CWE/CWE-131
-    @name Not enough memory allocated for array of pointer type (CWE-131)
+# CWE-131: Incorrect Calculation of Buffer Size
++ semmlecode-cpp-queries/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql: /CWE/CWE-131
+    @name No space for zero terminator (CWE-131)
++ semmlecode-cpp-queries/Critical/SizeCheck.ql: /CWE/CWE-131
+    @name Not enough memory allocated for pointer type (CWE-131)
++ semmlecode-cpp-queries/Critical/SizeCheck2.ql: /CWE/CWE-131
+    @name Not enough memory allocated for array of pointer type (CWE-131)

cpp/config/suites/security/cwe-134

@@ -1,13 +1,13 @@
-# CWE-134: Uncontrolled Format String
-+ semmlecode-cpp-queries/Likely Bugs/Format/NonConstantFormat.ql: /CWE/CWE-134
-    @name Non-constant format string (CWE-134)
-# This one runs out of memory. See ODASA-608.
-#+ semmlecode-cpp-queries/PointsTo/TaintedFormatStrings.ql: /CWE/CWE-134
-+ semmlecode-cpp-queries/Likely Bugs/Format/WrongNumberOfFormatArguments.ql: /CWE/CWE-134
-    @name Wrong number of arguments to formatting function (CWE-134)
-+ semmlecode-cpp-queries/Likely Bugs/Format/WrongTypeFormatArguments.ql: /CWE/CWE-134
-    @name Wrong type of arguments to formatting function (CWE-134)
-+ semmlecode-cpp-queries/Security/CWE/CWE-134/UncontrolledFormatString.ql: /CWE/CWE-134
-    @name Uncontrolled format string (CWE-134)
-+ semmlecode-cpp-queries/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql: /CWE/CWE-134
-    @name Uncontrolled format string (through global variable) (CWE-134)
+# CWE-134: Uncontrolled Format String
++ semmlecode-cpp-queries/Likely Bugs/Format/NonConstantFormat.ql: /CWE/CWE-134
+    @name Non-constant format string (CWE-134)
+# This one runs out of memory. See ODASA-608.
+#+ semmlecode-cpp-queries/PointsTo/TaintedFormatStrings.ql: /CWE/CWE-134
++ semmlecode-cpp-queries/Likely Bugs/Format/WrongNumberOfFormatArguments.ql: /CWE/CWE-134
+    @name Wrong number of arguments to formatting function (CWE-134)
++ semmlecode-cpp-queries/Likely Bugs/Format/WrongTypeFormatArguments.ql: /CWE/CWE-134
+    @name Wrong type of arguments to formatting function (CWE-134)
++ semmlecode-cpp-queries/Security/CWE/CWE-134/UncontrolledFormatString.ql: /CWE/CWE-134
+    @name Uncontrolled format string (CWE-134)
++ semmlecode-cpp-queries/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql: /CWE/CWE-134
+    @name Uncontrolled format string (through global variable) (CWE-134)

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

@@ -1,17 +1,17 @@
-// an include declaration just adds one source dependency, it does not automatically
-// add a dependency from this file to all the declarations in stdio.h
-#include <stdio.h>
-#include <myfile.h> // contains non-static global myfile_err
-
-extern int myfile_err; // this external declaration adds a dependency on myfile.h
-
-class C {
-public:
-	C() {
-		// one dependency for printf:
-		printf("Hello world!");
-		// one dependency for FILE type, and one for NULL macro:
-		FILE fp = NULL;
-	}
-};
-
+// an include declaration just adds one source dependency, it does not automatically
+// add a dependency from this file to all the declarations in stdio.h
+#include <stdio.h>
+#include <myfile.h> // contains non-static global myfile_err
+
+extern int myfile_err; // this external declaration adds a dependency on myfile.h
+
+class C {
+public:
+	C() {
+		// one dependency for printf:
+		printf("Hello world!");
+		// one dependency for FILE type, and one for NULL macro:
+		FILE fp = NULL;
+	}
+};
+

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

@@ -1,20 +1,20 @@
-//This struct contains 30 fields.
-struct MyParticle {
-  bool isActive;
-  int priority;
-
-  float x, y, z;
-  float dx, dy, dz;
-  float ddx, ddy, ddz;
-  bool isCollider;
-
-  int age, maxAge;
-  float size1, size2;
-
-  bool hasColor;
-  unsigned char r1, g1, b1, a1;
-  unsigned char r2, g2, b2, a2;
-
-  class texture *tex;
-  float u1, v1, u2, v2;
-};
+//This struct contains 30 fields.
+struct MyParticle {
+  bool isActive;
+  int priority;
+
+  float x, y, z;
+  float dx, dy, dz;
+  float ddx, ddy, ddz;
+  bool isCollider;
+
+  int age, maxAge;
+  float size1, size2;
+
+  bool hasColor;
+  unsigned char r1, g1, b1, a1;
+  unsigned char r2, g2, b2, a2;
+
+  class texture *tex;
+  float u1, v1, u2, v2;
+};

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

@@ -1,8 +1,8 @@
-// this example has 15 parameters.
-void fillRect(int x, int y, int w, int h,
-              int r1, int g1, int b1, int a1,
-              int r2, int g2, int b2, int a2,
-              gradient_type grad, unsigned int flags, bool border)
-{
-	// ...
-}
+// this example has 15 parameters.
+void fillRect(int x, int y, int w, int h,
+              int r1, int g1, int b1, int a1,
+              int r2, int g2, int b2, int a2,
+              gradient_type grad, unsigned int flags, bool border)
+{
+	// ...
+}

cpp/ql/src/Best Practices/ComplexCondition.cpp

@@ -1,13 +1,13 @@
-//This condition is too complex and can be improved by using local variables
-bool accept_message =
-	(message_type == CONNECT && _state != CONNECTED) ||
-	(message_type == DISCONNECT && _state == CONNECTED) ||
-	(message_type == DATA && _state == CONNECTED);
-
-//This condition is acceptable, as all the logical operators are of the same type (&&)
-bool valid_connect =
-	message_type == CONNECT && 
-	_state != CONNECTED &&
-	time_since_prev_connect > MAX_CONNECT_INTERVAL &&
-	message_length <= MAX_PACKET_SIZE &&
+//This condition is too complex and can be improved by using local variables
+bool accept_message =
+	(message_type == CONNECT && _state != CONNECTED) ||
+	(message_type == DISCONNECT && _state == CONNECTED) ||
+	(message_type == DATA && _state == CONNECTED);
+
+//This condition is acceptable, as all the logical operators are of the same type (&&)
+bool valid_connect =
+	message_type == CONNECT && 
+	_state != CONNECTED &&
+	time_since_prev_connect > MAX_CONNECT_INTERVAL &&
+	message_length <= MAX_PACKET_SIZE &&
 	checksum(message) == get_checksum_field(message);

cpp/ql/src/Best Practices/Hiding/DeclarationHidesParameter.cpp

@@ -1,6 +1,6 @@
-void f(int i) {
-  for (int i = 0; i < 10; ++i) { //the loop variable hides the parameter to f()
-    ...
-  }
-}
-
+void f(int i) {
+  for (int i = 0; i < 10; ++i) { //the loop variable hides the parameter to f()
+    ...
+  }
+}
+

cpp/ql/src/Best Practices/Hiding/DeclarationHidesVariable.cpp

@@ -1,12 +1,12 @@
-void f() {
-	int i = 10;
-
-	for (int i = 0; i < 10; i++) { //the loop counter hides the variable
-		 ...
-	}
-
-	{
-		int i = 12; //this variable hides the variable in the outer block
-		...
-	}
-}
+void f() {
+	int i = 10;
+
+	for (int i = 0; i < 10; i++) { //the loop counter hides the variable
+		 ...
+	}
+
+	{
+		int i = 12; //this variable hides the variable in the outer block
+		...
+	}
+}