Merge pull request #52 from kraiouchkine/kraiouchkine-dev-expressions

Implement C Expressions package and Fix RULE-11-1 false-negative

Author: Nikita Kraiouchkine

c/cert/src/rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.md

@@ -0,0 +1,70 @@
+/**
+ * @id c/cert/call-posix-open-with-correct-argument-count
+ * @name EXP37-C: Pass the correct number of arguments to the POSIX open function.
+ * @description A third argument should be passed to the POSIX function open() when and only when
+ *              creating a new file.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/exp37-c
+ *       correctness
+ *       security
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import semmle.code.cpp.commons.unix.Constants
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+
+int o_creat_val() {
+  result = any(MacroInvocation ma | ma.getMacroName() = "O_CREAT" | ma.getExpr().getValue().toInt())
+  or
+  result = o_creat()
+}
+
+/**
+ * A function call to POSIX open()
+ */
+class POSIXOpenFunctionCall extends FunctionCall {
+  POSIXOpenFunctionCall() { this.getTarget().getQualifiedName() = "open" }
+
+  /**
+   * Holds if reasonable bounds exist for the value of the 'flags' argument of the call.
+   * This predicate will never hold for cases such as wrapper functions
+   * which pass a parameter to the open() 'flags' argument.
+   */
+  predicate hasFlagsArgBounds() { lowerBound(this.getArgument(1)) >= 0 }
+
+  /**
+   * Holds if the 'flags' argument contains the O_CREAT flag.
+   * Because this predicate uses the SimpleRangeAnalysis library, it only
+   * analyzes the bounds of 'flag' arguments which can be deduced locally.
+   */
+  predicate isOpenCreateCall() {
+    hasFlagsArgBounds() and
+    upperBound(this.getArgument(1)).(int).bitAnd(o_creat_val()) != 0
+  }
+}
+
+from POSIXOpenFunctionCall call, string message
+where
+  not isExcluded(call, ExpressionsPackage::callPOSIXOpenWithCorrectArgumentCountQuery()) and
+  // include only analyzable flag arguments which have values that can be determined locally
+  call.hasFlagsArgBounds() and
+  // differentiate between two variants:
+  // 1) a call to open() with the O_CREAT flag set, which should pass three arguments
+  // 2) a call to open without the O_CREAT flag set, which should pass two arguments
+  if call.isOpenCreateCall()
+  then (
+    call.getNumberOfArguments() != 3 and
+    message =
+      "Call to " + call.getTarget().getName() +
+        " with O_CREAT flag does not pass exactly three arguments."
+  ) else (
+    call.getNumberOfArguments() != 2 and
+    message =
+      "Call to " + call.getTarget().getName() +
+        " without O_CREAT flag does not pass exactly two arguments."
+  )
+select call, message

c/cert/src/rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.ql

@@ -0,0 +1,62 @@
+/**
+ * @id c/cert/do-not-call-function-pointer-with-incompatible-type
+ * @name EXP37-C: Do not call a function pointer which points to a function of an incompatible type
+ * @description Calling a function pointer with a type incompatible with the function it points to
+ *              is undefined.
+ * @kind path-problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/exp37-c
+ *       correctness
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import semmle.code.cpp.dataflow.DataFlow
+import DataFlow::PathGraph
+
+/**
+ * An expression of type `FunctionPointer` which is the unconverted expression of a cast
+ * which converts the function pointer to a pointer to a function of a different type.
+ */
+class SuspiciousFunctionPointerCastExpr extends Expr {
+  SuspiciousFunctionPointerCastExpr() {
+    exists(CStyleCast cast, Type old, Type new |
+      this = cast.getUnconverted() and
+      old = cast.getUnconverted().getUnderlyingType() and
+      new = cast.getFullyConverted().getUnderlyingType() and
+      old != new and
+      old instanceof FunctionPointerType and
+      new instanceof FunctionPointerType
+    )
+  }
+}
+
+/**
+ * Data-flow configuration for flow from a `SuspiciousFunctionPointerCastExpr`
+ * to a call of the function pointer resulting from the function pointer cast
+ */
+class SuspectFunctionPointerToCallConfig extends DataFlow::Configuration {
+  SuspectFunctionPointerToCallConfig() { this = "SuspectFunctionPointerToCallConfig" }
+
+  override predicate isSource(DataFlow::Node src) {
+    src.asExpr() instanceof SuspiciousFunctionPointerCastExpr
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(VariableCall call | sink.asExpr() = call.getExpr().(VariableAccess))
+  }
+}
+
+from
+  SuspectFunctionPointerToCallConfig config, DataFlow::PathNode src, DataFlow::PathNode sink,
+  Access access
+where
+  not isExcluded(src.getNode().asExpr(),
+    ExpressionsPackage::doNotCallFunctionPointerWithIncompatibleTypeQuery()) and
+  access = src.getNode().asExpr() and
+  config.hasFlowPath(src, sink)
+select src, src, sink,
+  "Incompatible function $@ assigned to function pointer is eventually called through the pointer.",
+  access.getTarget(), access.getTarget().getName()

c/cert/src/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.md

@@ -0,0 +1,28 @@
+/**
+ * @id c/cert/do-not-call-functions-with-incompatible-arguments
+ * @name EXP37-C: Do not pass arguments with an incompatible count or type to a function.
+ * @description The arguments passed to a function must be compatible with the function's
+ *              parameters.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/exp37-c
+ *       correctness
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.MistypedFunctionArguments
+
+from FunctionCall fc, Function f, Parameter p
+where
+  not isExcluded(fc, ExpressionsPackage::doNotCallFunctionsWithIncompatibleArgumentsQuery()) and
+  (
+    mistypedFunctionArguments(fc, f, p)
+    or
+    complexArgumentPassedToRealParameter(fc, f, p)
+  )
+select fc,
+  "Argument $@ in call to " + f.toString() + " is incompatible with parameter " + p.getTypedName() +
+    ".", fc.getArgument(p.getIndex()) as arg, arg.toString()

c/cert/src/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql

@@ -0,0 +1,74 @@
+# EXP46-C: Do not use a bitwise operator with a Boolean-like operand
+
+This query implements the CERT-C rule EXP46-C:
+
+> Do not use a bitwise operator with a Boolean-like operand
+
+
+## Description
+
+Mixing bitwise and relational operators in the same full expression can be a sign of a logic error in the expression where a logical operator is usually the intended operator. Do not use the bitwise AND (`&`), bitwise OR (`|`), or bitwise XOR (`^`) operators with an operand of type `_Bool`, or the result of a *relational-expression* or *equality-expression*. If the bitwise operator is intended, it should be indicated with use of a parenthesized expression.
+
+## Noncompliant Code Example
+
+In this noncompliant code example, a bitwise `&` operator is used with the results of an *equality-expression*:
+
+```cpp
+if (!(getuid() & geteuid() == 0)) { 
+  /* ... */ 
+} 
+
+```
+
+## Compliant Solution
+
+This compliant solution uses the `&&` operator for the logical operation within the conditional expression:
+
+```cpp
+if (!(getuid() && geteuid() == 0)) {
+  /* ... */
+}
+
+```
+
+## Risk Assessment
+
+<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> EXP46-C </td> <td> Low </td> <td> Likely </td> <td> Low </td> <td> <strong>P9</strong> </td> <td> <strong>L2</strong> </td> </tr> </tbody> </table>
+
+
+## Automated Detection
+
+<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Astrée </a> </td> <td> 22.04 </td> <td> <strong>inappropriate-bool</strong> </td> <td> Supported indirectly via MISRA C:2012 Rule 10.1 </td> </tr> <tr> <td> <a> Axivion Bauhaus Suite </a> </td> <td> 7.2.0 </td> <td> <strong>CertC-EXP46</strong> </td> <td> </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>LANG.TYPE.IOT</strong> </td> <td> Inappropriate operand type </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>CONSTANT_EXPRESSION_RESULT</strong> </td> <td> Partially implemented </td> </tr> <tr> <td> <a> Cppcheck </a> </td> <td> 1.66 </td> <td> cert.py </td> <td> Detected by the addon cert.py </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C3344, C4502</strong> <strong>C++3709</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.2 </td> <td> <strong>MISRA.LOGIC.OPERATOR.NOT_BOOL</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>136 S</strong> </td> <td> Fully Implemented </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-EXP46-b</strong> </td> <td> Expressions that are effectively Boolean should not be used as operands to operators other than (&amp;&amp;, ||, !, =, ==, !=, ?:) </td> </tr> <tr> <td> <a> PC-lint Plus </a> </td> <td> 1.4 </td> <td> <strong>514</strong> </td> <td> Fully supported </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule EXP46-C </a> </td> <td> Checks for bitwise operations on boolean operands (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>3344,4502</strong> </td> <td> </td> </tr> <tr> <td> <a> PRQA QA-C++ </a> </td> <td> 4.4 </td> <td> <strong>3709</strong> </td> <td> </td> </tr> <tr> <td> <a> PVS-Studio </a> </td> <td> 7.20 </td> <td> <strong><a>V564</a>, <a>V1015</a></strong> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 22.04 </td> <td> <strong>inappropriate-bool</strong> </td> <td> Supported indirectly via MISRA C:2012 Rule 10.1 </td> </tr> </tbody> </table>
+
+
+## Related Guidelines
+
+[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
+
+<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> ISO/IEC TR 24772:2013 </a> </td> <td> Likely Incorrect Expression \[KOA\] </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> <tr> <td> <a> CWE 2.11 </a> </td> <td> <a> CWE-480 </a> , Use of incorrect operator </td> <td> 2017-07-05: CERT: Rule subset of CWE </td> </tr> <tr> <td> <a> CWE 2.11 </a> </td> <td> <a> CWE-569 </a> </td> <td> 2017-07-06: CERT: Rule subset of CWE </td> </tr> </tbody> </table>
+
+
+## CERT-CWE Mapping Notes
+
+[Key here](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152408#HowthisCodingStandardisOrganized-CERT-CWEMappingNotes) for mapping notes
+
+**CWE-480 and EXP46-C**
+
+Intersection( EXP45-C, EXP46-C) = Ø
+
+CWE-480 = Union( EXP46-C, list) where list =
+
+* Usage of incorrect operator besides s/&amp;/&amp;&amp;/ or s/|/||/
+
+## Bibliography
+
+<table> <tbody> <tr> <td> \[ <a> Hatton 1995 </a> \] </td> <td> Section 2.7.2, "Errors of Omission and Addition" </td> </tr> </tbody> </table>
+
+
+## Implementation notes
+
+None
+
+## References
+
+* CERT-C: [EXP46-C: Do not use a bitwise operator with a Boolean-like operand](https://wiki.sei.cmu.edu/confluence/display/c)

c/cert/src/rules/EXP37-C/DoNotCallFunctionsWithIncompatibleArguments.md

@@ -0,0 +1,49 @@
+/**
+ * @id c/cert/do-not-use-a-bitwise-operator-with-a-boolean-like-operand
+ * @name EXP46-C: Do not use a bitwise operator with a Boolean-like operand
+ * @description Using bitwise operators with unparenthesized Boolean-like operands may indicate a
+ *              logic error.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/cert/id/exp46-c
+ *       maintainability
+ *       readability
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+
+/**
+ * Holds if `op` is a bitwise AND, OR, or XOR expression
+ */
+predicate isBitwiseOperationPotentiallyAmbiguous(BinaryBitwiseOperation op) {
+  op instanceof BitwiseAndExpr or
+  op instanceof BitwiseOrExpr or
+  op instanceof BitwiseXorExpr
+}
+
+/**
+ * Holds if `e` is an unparenthesised boolean expression,
+ * relational operation, or equality operation.
+ */
+predicate isDisallowedBitwiseOperationOperand(Expr e) {
+  not e.isParenthesised() and
+  (
+    e.getFullyConverted().getUnderlyingType() instanceof BoolType or
+    e instanceof RelationalOperation or
+    e instanceof EqualityOperation
+  )
+}
+
+from Expr operand, Operation operation
+where
+  not isExcluded(operation,
+    ExpressionsPackage::doNotUseABitwiseOperatorWithABooleanLikeOperandQuery()) and
+  isBitwiseOperationPotentiallyAmbiguous(operation) and
+  operand = operation.getAnOperand() and
+  isDisallowedBitwiseOperationOperand(operand)
+select operation,
+  "Bitwise operator " + operation.getOperator() +
+    " performs potentially unintended operation on $@.", operand, "boolean operand"

c/cert/src/rules/EXP37-C/DoNotCallFunctionsWithIncompatibleArguments.ql

@@ -0,0 +1,6 @@
+| test.c:18:3:18:6 | call to open | Call to open without O_CREAT flag does not pass exactly two arguments. |
+| test.c:19:3:19:6 | call to open | Call to open with O_CREAT flag does not pass exactly three arguments. |
+| test.c:23:3:23:6 | call to open | Call to open without O_CREAT flag does not pass exactly two arguments. |
+| test.c:26:3:26:6 | call to open | Call to open with O_CREAT flag does not pass exactly three arguments. |
+| test.c:34:3:34:6 | call to open | Call to open without O_CREAT flag does not pass exactly two arguments. |
+| test.c:35:3:35:6 | call to open | Call to open with O_CREAT flag does not pass exactly three arguments. |

c/cert/src/rules/EXP46-C/DoNotUseABitwiseOperatorWithABooleanLikeOperand.md

@@ -0,0 +1 @@
+rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.ql

c/cert/src/rules/EXP46-C/DoNotUseABitwiseOperatorWithABooleanLikeOperand.ql

@@ -0,0 +1,24 @@
+edges
+| test.c:48:68:48:70 | fns [f1] | test.c:49:3:49:5 | fns [f1] |
+| test.c:49:3:49:5 | fns [f1] | test.c:49:8:49:9 | f1 |
+| test.c:61:28:61:29 | f2 | test.c:62:3:62:11 | v1_called |
+| test.c:73:3:73:5 | fns [post update] [f1] | test.c:75:45:75:48 | & ... [f1] |
+| test.c:73:3:73:13 | ... = ... | test.c:73:3:73:5 | fns [post update] [f1] |
+| test.c:73:12:73:13 | v2 | test.c:73:3:73:13 | ... = ... |
+| test.c:75:45:75:48 | & ... [f1] | test.c:48:68:48:70 | fns [f1] |
+nodes
+| test.c:48:68:48:70 | fns [f1] | semmle.label | fns [f1] |
+| test.c:49:3:49:5 | fns [f1] | semmle.label | fns [f1] |
+| test.c:49:8:49:9 | f1 | semmle.label | f1 |
+| test.c:61:28:61:29 | f2 | semmle.label | f2 |
+| test.c:62:3:62:11 | v1_called | semmle.label | v1_called |
+| test.c:70:9:70:17 | v3_called | semmle.label | v3_called |
+| test.c:73:3:73:5 | fns [post update] [f1] | semmle.label | fns [post update] [f1] |
+| test.c:73:3:73:13 | ... = ... | semmle.label | ... = ... |
+| test.c:73:12:73:13 | v2 | semmle.label | v2 |
+| test.c:75:45:75:48 | & ... [f1] | semmle.label | & ... [f1] |
+subpaths
+#select
+| test.c:61:28:61:29 | f2 | test.c:61:28:61:29 | f2 | test.c:62:3:62:11 | v1_called | Incompatible function $@ assigned to function pointer is eventually called through the pointer. | test.c:41:13:41:14 | f2 | f2 |
+| test.c:70:9:70:17 | v3_called | test.c:70:9:70:17 | v3_called | test.c:70:9:70:17 | v3_called | Incompatible function $@ assigned to function pointer is eventually called through the pointer. | test.c:58:7:58:15 | v3_called | v3_called |
+| test.c:73:12:73:13 | v2 | test.c:73:12:73:13 | v2 | test.c:49:8:49:9 | f1 | Incompatible function $@ assigned to function pointer is eventually called through the pointer. | test.c:56:7:56:8 | v2 | v2 |

c/cert/test/rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.expected

@@ -0,0 +1 @@
+rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql

c/cert/test/rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.qlref

@@ -0,0 +1,3 @@
+| test.c:83:12:83:16 | call to atan2 | Argument $@ in call to atan2 is incompatible with parameter double (unnamed parameter 0). | test.c:83:18:83:18 | c | c |
+| test.c:93:3:93:12 | call to test_func1 | Argument $@ in call to test_func1 is incompatible with parameter short p1. | test.c:93:14:93:15 | p1 | p1 |
+| test.c:94:3:94:12 | call to test_func1 | Argument $@ in call to test_func1 is incompatible with parameter short p1. | test.c:94:14:94:15 | p2 | p2 |

c/cert/test/rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.expected

@@ -0,0 +1 @@
+rules/EXP37-C/DoNotCallFunctionsWithIncompatibleArguments.ql