Merge pull request #1136 from zlaski-semmle/cpp340a

[CPP-340] Refinements to FutileParams.ql etc.

Author: jbj

change-notes/1.21/analysis-cpp.md

@@ -6,6 +6,8 @@
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
+| `()`-declared function called with too few arguments (`cpp/too-few-arguments`) | Correctness | Find all cases where the number of arguments is less than the number of parameters of the function, provided the function is also properly declared/defined elsewhere. |
+| `()`-declared function called with mismatched arguments (`cpp/mismatched-function-arguments`) | Correctness | Find all cases where the types of arguments do not match the types of parameters of the function, provided the function is also properly declared/defined elsewhere. |
 
 ## Changes to existing queries
 
@@ -24,6 +26,7 @@
 | Use of potentially dangerous function | More correct results | Calls to `localtime`, `ctime` and `asctime` are now detected by this query. |
 | Wrong type of arguments to formatting function (`cpp/wrong-type-format-argument`) | More correct results and fewer false positive results | This query now more accurately identifies wide and non-wide string/character format arguments on different platforms.  Platform detection has also been made more accurate for the purposes of this query. |
 | Wrong type of arguments to formatting function (`cpp/wrong-type-format-argument`) | Fewer false positive results | Non-standard uses of %L are now understood. |
+| `()`-declared function called with too many arguments (`cpp/futile-params`) | Improved coverage | Query has been generalized to find all cases where the number of arguments exceedes the number of parameters of the function, provided the function is also properly declared/defined elsewhere. |
 
 ## Changes to QL libraries
 - Additional support for definition by reference has been added to the `semmle.code.cpp.dataflow.TaintTracking` library.

cpp/config/suites/c/correctness

@@ -6,6 +6,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Arithmetic/IntMultToLong.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
++ semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
@@ -15,7 +16,8 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/AssignWhereCompareMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/CompareWhereAssignMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
-+ semmlecode-cpp-queries/Likely Bugs/Likely Typos/FutileParams.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/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

@@ -7,6 +7,7 @@
 + semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
++ semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
@@ -16,7 +17,8 @@
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/AssignWhereCompareMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/CompareWhereAssignMeant.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Likely Bugs/Likely Typos/ExprHasNoEffect.ql: /Correctness/Common Errors
-+ semmlecode-cpp-queries/Likely Bugs/Likely Typos/FutileParams.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/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/Likely Typos/FutileParams.c

@@ -0,0 +1,7 @@
+void three_arguments(int x, int y, int z);
+
+void calls() {
+	int three = 3;
+	three_arguments(1, 2, three); // GOOD
+	three_arguments(1, 2, &three); // BAD
+}

cpp/ql/src/Likely Bugs/Likely Typos/FutileParams.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>A function is called with at least one argument whose type is incompatible with the type of
+ the corresponding parameter of the function being called.  This may cause the called function
+ to behave unpredictably.</p>
+
+<p>This may indicate that an incorrect function is being called, or that the
+ signature (parameter list and parameter types) of the called function 
+ is not known to the author.</p>
+
+</overview>
+<recommendation>
+<p>Call the function with the proper argument types.  In some cases, it may
+ suffice to provide an explicit cast of an argument to the desired (parameter) type.</p>
+
+</recommendation>
+<example><sample src="MistypedFunctionArguments.c" />
+
+</example>
+
+<references>
+<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/DCL20-C.+Explicitly+specify+void+when+a+function+accepts+no+arguments"> DCL20-C. Explicitly specify void when a function accepts no arguments </a></li>
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Likely Typos/FutileParams.ql

@@ -0,0 +1,108 @@
+/**
+ * @name Call to a function with one or more incompatible arguments
+ * @description When the type of a function argument is not compatible
+ * with the type of the corresponding parameter, it may lead to
+ * unpredictable behavior.
+ * @kind problem
+ * @problem.severity warning
+ * @precision medium
+ * @id cpp/mistyped-function-arguments
+ * @tags correctness
+ *       maintainability
+ */
+
+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().getType().getUnspecifiedType(),
+    parm.getType().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(Function f) {
+  exists(File file | file.compiledAsC() |
+    file = f.getFile() or file.getAnIncludedFile+() = f.getFile()
+  )
+}
+
+from FunctionCall fc, Function f, Parameter p
+where
+  f = fc.getTarget() and
+  p = f.getAParameter() and
+  hasZeroParamDecl(f) and
+  isCompiledAsC(f) 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)
+select fc, "Calling $@: argument $@ of type $@ is incompatible with parameter $@.", f, f.toString(),
+  fc.getArgument(p.getIndex()) as arg, arg.toString(),
+  arg.getExplicitlyConverted().getType().getUnspecifiedType() as atype, atype.toString(), p,
+  p.getTypedName()

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

@@ -0,0 +1,9 @@
+void one_argument();
+
+void calls() {
+	one_argument(1); // GOOD: `one_argument` will accept and use the argument
+	
+	one_argument(); // BAD: `one_argument` will receive an undefined value
+}
+
+void one_argument(int x);