Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 243 additions & 0 deletions c/cert/src/rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.md

Large diffs are not rendered by default.

70 changes: 70 additions & 0 deletions c/cert/src/rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.ql
Original file line number Diff line number Diff line change
@@ -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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -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()

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -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()
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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) {
Comment thread
mbaluda marked this conversation as resolved.
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"
Original file line number Diff line number Diff line change
@@ -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. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/EXP37-C/CallPOSIXOpenWithCorrectArgumentCount.ql
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/EXP37-C/DoNotCallFunctionPointerWithIncompatibleType.ql
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/EXP37-C/DoNotCallFunctionsWithIncompatibleArguments.ql
Loading