Merge from master

Author: Dave Bartolomeo

change-notes/1.24/analysis-cpp.md

@@ -8,6 +8,7 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
+| Implicit function declarations (`cpp/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql`) | correctness, maintainability | This query finds calls to undeclared functions that are compiled by a C compiler. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
 

change-notes/1.24/analysis-csharp.md

@@ -14,6 +14,7 @@ The following changes in version 1.24 affect C# analysis in all applications.
 | **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
 | Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the variable is named `_` in a `foreach` statement. | 
+| Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. |
 
 ## Removal of old queries
 

change-notes/1.24/analysis-java.md

@@ -22,3 +22,10 @@ The following changes in version 1.24 affect Java analysis in all applications.
 
 ## Changes to libraries
 
+* 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.

change-notes/1.24/analysis-javascript.md

@@ -13,6 +13,9 @@
   - [react](https://www.npmjs.com/package/react)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
   - [Handlebars](https://www.npmjs.com/package/handlebars)
+  - [Electron](https://electronjs.org/)
+  - [Node.js](https://nodejs.org/)
+  - [Socket.IO](https://socket.io/)
 
 ## New queries
 
@@ -37,3 +40,4 @@
 ## Changes to libraries
 
 * The predicates `RegExpTerm.getSuccessor` and `RegExpTerm.getPredecessor` have been changed to reflect textual, not operational, matching order. This only makes a difference in lookbehind assertions, which are operationally matched backwards. Previously, `getSuccessor` would mimick this, so in an assertion `(?<=ab)` the term `b` would be considered the predecessor, not the successor, of `a`. Textually, however, `a` is still matched before `b`, and this is the order we now follow.
+* An extensible model of the `EventEmitter` pattern has been implemented.

cpp/config/suites/c/correctness

@@ -18,6 +18,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooFewArguments.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooManyArguments.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ShortCircuitBitMask.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/FloatComparison.ql: /Correctness/Common Errors

cpp/config/suites/cpp/correctness

@@ -19,6 +19,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooFewArguments.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/TooManyArguments.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ShortCircuitBitMask.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/FloatComparison.ql: /Correctness/Common Errors

cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.c

@@ -0,0 +1,8 @@
+/* '#include <stdlib.h>' was forgotton */
+
+int main(void) {
+	/* 'int malloc()' assumed */
+	unsigned char *p = malloc(100);
+	*p = 'a';
+	return 0;
+}

cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>A function is called without a prior function declaration or definition.
+When this happens, the compiler generates an implicit declaration of the function,
+specifying an integer return type and no parameters.
+If the implicit declaration does not match the true signature of the function, the
+function may behave unpredictably.</p>
+
+<p>This may indicate a misspelled function name, or that the required header containing
+the function declaration has not been included.</p>
+
+</overview>
+<recommendation>
+<p>Provide an explicit declaration of the function before invoking it.</p>
+
+</recommendation>
+<example><sample src="ImplicitFunctionDeclaration.c" />
+
+</example>
+
+<references>
+<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/DCL31-C.+Declare+identifiers+before+using+them">DCL31-C. Declare identifiers before using them</a></li>
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Underspecified Functions/ImplicitFunctionDeclaration.ql

@@ -0,0 +1,48 @@
+/**
+ * @name Implicit function declaration
+ * @description An implicitly declared function is assumed to take no
+ * arguments and return an integer. If this assumption does not hold, it
+ * may lead to unpredictable behavior.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id cpp/implicit-function-declaration
+ * @tags correctness
+ *       maintainability
+ */
+
+import cpp
+import MistypedFunctionArguments
+import TooFewArguments
+import TooManyArguments
+import semmle.code.cpp.commons.Exclusions
+
+predicate locInfo(Locatable e, File file, int line, int col) {
+  e.getFile() = file and
+  e.getLocation().getStartLine() = line and
+  e.getLocation().getStartColumn() = col
+}
+
+predicate sameLocation(FunctionDeclarationEntry fde, FunctionCall fc) {
+  exists(File file, int line, int col |
+    locInfo(fde, file, line, col) and
+    locInfo(fc, file, line, col)
+  )
+}
+
+predicate isCompiledAsC(File f) {
+  f.compiledAsC()
+  or
+  exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
+}
+
+from FunctionDeclarationEntry fdeIm, FunctionCall fc
+where
+  isCompiledAsC(fdeIm.getFile()) and
+  not isFromMacroDefinition(fc) and
+  fdeIm.isImplicit() and
+  sameLocation(fdeIm, fc) and
+  not mistypedFunctionArguments(fc, _, _) and
+  not tooFewArguments(fc, _) and
+  not tooManyArguments(fc, _)
+select fc, "Function call implicitly declares '" + fdeIm.getName() + "'."

cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql

@@ -12,95 +12,10 @@
  */
 
 import cpp
-
-predicate arithTypesMatch(Type arg, Type parm) {
-  arg = parm
-  or
-  arg.getSize() = parm.getSize() and
-  (
-    arg instanceof IntegralOrEnumType and
-    parm instanceof IntegralOrEnumType
-    or
-    arg instanceof FloatingPointType and
-    parm instanceof FloatingPointType
-  )
-}
-
-pragma[inline]
-predicate nestedPointerArgTypeMayBeUsed(Type arg, Type parm) {
-  // arithmetic types
-  arithTypesMatch(arg, parm)
-  or
-  // conversion to/from pointers to void is allowed
-  arg instanceof VoidType
-  or
-  parm instanceof VoidType
-}
-
-pragma[inline]
-predicate pointerArgTypeMayBeUsed(Type arg, Type parm) {
-  nestedPointerArgTypeMayBeUsed(arg, parm)
-  or
-  // nested pointers
-  nestedPointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
-    parm.(PointerType).getBaseType().getUnspecifiedType())
-  or
-  nestedPointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
-    parm.(PointerType).getBaseType().getUnspecifiedType())
-}
-
-pragma[inline]
-predicate argTypeMayBeUsed(Type arg, Type parm) {
-  // arithmetic types
-  arithTypesMatch(arg, parm)
-  or
-  // pointers to compatible types
-  pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
-    parm.(PointerType).getBaseType().getUnspecifiedType())
-  or
-  pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
-    parm.(PointerType).getBaseType().getUnspecifiedType())
-  or
-  // C11 arrays
-  pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
-    parm.(ArrayType).getBaseType().getUnspecifiedType())
-  or
-  pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
-    parm.(ArrayType).getBaseType().getUnspecifiedType())
-}
-
-// This predicate holds whenever expression `arg` may be used to initialize
-// function parameter `parm` without need for run-time conversion.
-pragma[inline]
-predicate argMayBeUsed(Expr arg, Parameter parm) {
-  argTypeMayBeUsed(arg.getFullyConverted().getUnspecifiedType(), parm.getUnspecifiedType())
-}
-
-// True if function was ()-declared, but not (void)-declared or K&R-defined
-predicate hasZeroParamDecl(Function f) {
-  exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
-    not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
-  )
-}
-
-// True if this file (or header) was compiled as a C file
-predicate isCompiledAsC(File f) {
-  f.compiledAsC()
-  or
-  exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
-}
+import MistypedFunctionArguments
 
 from FunctionCall fc, Function f, Parameter p
-where
-  f = fc.getTarget() and
-  p = f.getAParameter() and
-  hasZeroParamDecl(f) and
-  isCompiledAsC(f.getFile()) and
-  not f.isVarargs() and
-  not f instanceof BuiltInFunction and
-  p.getIndex() < fc.getNumberOfArguments() and
-  // Parameter p and its corresponding call argument must have mismatched types
-  not argMayBeUsed(fc.getArgument(p.getIndex()), p)
+where mistypedFunctionArguments(fc, f, p)
 select fc, "Calling $@: argument $@ of type $@ is incompatible with parameter $@.", f, f.toString(),
   fc.getArgument(p.getIndex()) as arg, arg.toString(),
   arg.getExplicitlyConverted().getUnspecifiedType() as atype, atype.toString(), p, p.getTypedName()