Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Implement expressions2 package#881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
MichaelRFairhurst wants to merge12 commits intomain
base:main
Choose a base branch
Loading
frommichaelrfairhurst/implement-expressions2-package
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
12 commits
Select commitHold shift + click to select a range
d2e638e
Implement expressions2 package
MichaelRFairhurstMar 31, 2025
f6d62ec
Format
MichaelRFairhurstMar 31, 2025
067e172
Change obligation
MichaelRFairhurstApr 10, 2025
010b7c7
Remove commented out code
MichaelRFairhurstApr 10, 2025
b0033df
Merge branch 'main' into michaelrfairhurst/implement-expressions2-pac…
lcarteyMay 10, 2025
6a5aa1f
Fix compilation error
MichaelRFairhurstJun 13, 2025
bf0472d
Merge remote-tracking branch 'origin/main' into michaelrfairhurst/imp…
MichaelRFairhurstJun 16, 2025
df1bd3a
Add missing CERT-C query tags
MichaelRFairhurstJun 16, 2025
bb7c913
Fix remediation cost
MichaelRFairhurstJun 16, 2025
fbf21bd
Undo accidental deletion of LeafType
MichaelRFairhurstJun 16, 2025
e4121ee
Remove cert c obligation tag
MichaelRFairhurstJun 16, 2025
e655ed8
Exclude if (f) f() from EXP-16-C
MichaelRFairhurstJun 19, 2025
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
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
# EXP16-C: Do not compare function pointers to constant values

This query implements the CERT-C rule EXP16-C:

> Do not compare function pointers to constant values


## Description

Comparing a function pointer to a value that is not a null function pointer of the same type will be diagnosed because it typically indicates programmer error and can result in [unexpected behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-unexpectedbehavior). Implicit comparisons will be diagnosed, as well.

## Noncompliant Code Example

In this noncompliant code example, the addresses of the POSIX functions `getuid` and `geteuid` are compared for equality to 0. Because no function address shall be null, the first subexpression will always evaluate to false (0), and the second subexpression always to true (nonzero). Consequently, the entire expression will always evaluate to true, leading to a potential security vulnerability.

```cpp
/* First the options that are allowed only for root */
if (getuid == 0 || geteuid != 0) {
/* ... */
}

```

## Noncompliant Code Example

In this noncompliant code example, the function pointers `getuid` and `geteuid` are compared to 0. This example is from an actual [vulnerability](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) ([VU\#837857](http://www.kb.cert.org/vuls/id/837857)) discovered in some versions of the X Window System server. The vulnerability exists because the programmer neglected to provide the open and close parentheses following the `geteuid()` function identifier. As a result, the `geteuid` token returns the address of the function, which is never equal to 0. Consequently, the `or` condition of this `if` statement is always true, and access is provided to the protected block for all users. Many compilers issue a warning noting such pointless expressions. Therefore, this coding error is normally detected by adherence to [MSC00-C. Compile cleanly at high warning levels](https://wiki.sei.cmu.edu/confluence/display/c/MSC00-C.+Compile+cleanly+at+high+warning+levels).

```cpp
/* First the options that are allowed only for root */
if (getuid() == 0 || geteuid != 0) {
/* ... */
}

```

## Compliant Solution

The solution is to provide the open and close parentheses following the `geteuid` token so that the function is properly invoked:

```cpp
/* First the options that are allowed only for root */
if (getuid() == 0 || geteuid() != 0) {
/* ... */
}

```

## Compliant Solution

A function pointer can be compared to a null function pointer of the same type:

```cpp
/* First the options that are allowed only for root */
if (getuid == (uid_t(*)(void))0 || geteuid != (uid_t(*)(void))0) {
/* ... */
}

```
This code should not be diagnosed by an analyzer.

## Noncompliant Code Example

In this noncompliant code example, the function pointer `do_xyz` is implicitly compared unequal to 0:

```cpp
int do_xyz(void);

int f(void) {
/* ... */
if (do_xyz) {
return -1; /* Indicate failure */
}
/* ... */
return 0;
}

```

## Compliant Solution

In this compliant solution, the function `do_xyz()` is invoked and the return value is compared to 0:

```cpp
int do_xyz(void);

int f(void) {
/* ... */
if (do_xyz()) {
return -1; /* Indicate failure */
}
/* ... */
return 0;
}

```

## Risk Assessment

Errors of omission can result in unintended program flow.

<table> <tbody> <tr> <th> Recommendation </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> EXP16-C </td> <td> Low </td> <td> Likely </td> <td> Medium </td> <td> <strong>P6</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> 24.04 </td> <td> <strong>function-name-constant-comparison</strong> </td> <td> Partially checked </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>BAD_COMPARE</strong> </td> <td> Can detect the specific instance where the address of a function is compared against 0, such as in the case of <code>geteuid</code> versus <code>getuid()</code> in the implementation-specific details </td> </tr> <tr> <td> <a> GCC </a> </td> <td> 4.3.5 </td> <td> </td> <td> Can detect violations of this recommendation when the <code>-Wall</code> flag is used </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2024.4 </td> <td> <strong>C0428, C3004, C3344</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2024.4 </td> <td> <strong>CWARN.NULLCHECK.FUNCNAMECWARN.FUNCADDR</strong> </td> <td> </td> </tr> <tr> <td> <a> LDRA tool suite </a> </td> <td> 9.7.1 </td> <td> <strong>99 S</strong> </td> <td> Partially implemented </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2024.2 </td> <td> <strong>CERT_C-EXP16-a</strong> </td> <td> Function address should not be compared to zero </td> </tr> <tr> <td> <a> PC-lint Plus </a> </td> <td> 1.4 </td> <td> <strong>2440, 2441</strong> </td> <td> Partially supported: reports address of function, array, or variable directly or indirectly compared to null </td> </tr> <tr> <td> <a> PVS-Studio </a> </td> <td> 7.35 </td> <td> <strong><a>V516</a>, <a>V1058</a></strong> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 24.04 </td> <td> <strong>function-name-constant-comparison</strong> </td> <td> Partially checked </td> </tr> </tbody> </table>


## Related Vulnerabilities

Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+EXP16-C).

## Related Guidelines

<table> <tbody> <tr> <td> <a> SEI CERT C++ Coding Standard </a> </td> <td> <a> VOID EXP16-CPP. Avoid conversions using void pointers </a> </td> </tr> <tr> <td> <a> ISO/IEC TR 24772:2013 </a> </td> <td> Likely incorrect expressions \[KOA\] </td> </tr> <tr> <td> <a> ISO/IEC TS 17961 </a> </td> <td> Comparing function addresses to zero \[funcaddr\] </td> </tr> <tr> <td> <a> MITRE CWE </a> </td> <td> <a> CWE-480 </a> , Use of incorrect operator <a> CWE-482 </a> , Comparing instead of assigning </td> </tr> </tbody> </table>


## 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: [EXP16-C: Do not compare function pointers to constant values](https://wiki.sei.cmu.edu/confluence/display/c)
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
/**
* @id c/cert/do-not-compare-function-pointers-to-constant-values
* @name EXP16-C: Do not compare function pointers to constant values
* @description Comparing function pointers to a constant value is not reliable and likely indicates
* a programmer error.
* @kind problem
* @precision very-high
* @problem.severity error
* @tags external/cert/id/exp16-c
* correctness
* external/cert/severity/low
* external/cert/likelihood/likely
* external/cert/remediation-cost/medium
* external/cert/priority/p6
* external/cert/level/l2
* external/cert/obligation/recommendation
*/

import cpp
import semmle.code.cpp.controlflow.IRGuards
import codingstandards.c.cert
import codingstandards.cpp.types.FunctionType
import codingstandards.cpp.exprs.FunctionExprs
import codingstandards.cpp.exprs.Guards

abstract class EffectivelyComparison extends Element {
abstract string getExplanation();

abstract FunctionExpr getFunctionExpr();
}

class ExplicitComparison extends EffectivelyComparison, ComparisonOperation {
Expr constantExpr;
FunctionExpr funcExpr;

ExplicitComparison() {
funcExpr = getAnOperand() and
constantExpr = getAnOperand() and
exists(constantExpr.getValue()) and
not funcExpr = constantExpr and
not constantExpr.getExplicitlyConverted().getUnderlyingType() =
funcExpr.getExplicitlyConverted().getUnderlyingType()
}

override string getExplanation() { result = "$@ compared to constant value." }

override FunctionExpr getFunctionExpr() { result = funcExpr }
}

class ImplicitComparison extends EffectivelyComparison, GuardCondition {
ImplicitComparison() {
this instanceof FunctionExpr and
not getParent() instanceof ComparisonOperation
}

override string getExplanation() { result = "$@ undergoes implicit constant comparison." }

override FunctionExpr getFunctionExpr() { result = this }
}

from EffectivelyComparison comparison, FunctionExpr funcExpr, Element function, string funcName
where
not isExcluded(comparison,
Expressions2Package::doNotCompareFunctionPointersToConstantValuesQuery()) and
funcExpr = comparison.getFunctionExpr() and
not exists(NullFunctionCallGuard nullGuard | nullGuard.getFunctionExpr() = funcExpr) and
function = funcExpr.getFunction() and
funcName = funcExpr.describe()
select comparison, comparison.getExplanation(), function, funcName
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
| test.c:17:7:17:13 | ... == ... | $@ compared to constant value. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:20:7:20:12 | ... > ... | $@ compared to constant value. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:29:7:29:13 | ... == ... | $@ compared to constant value. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
| test.c:32:7:32:16 | ... == ... | $@ compared to constant value. | test.c:5:7:5:8 | g2 | Function pointer variable g2 |
| test.c:35:7:35:15 | ... != ... | $@ compared to constant value. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
| test.c:38:7:38:8 | f1 | $@ undergoes implicit constant comparison. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:41:7:41:8 | g1 | $@ undergoes implicit constant comparison. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
| test.c:68:7:68:27 | ... == ... | $@ compared to constant value. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:71:7:71:18 | ... == ... | $@ compared to constant value. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:74:7:76:14 | ... == ... | $@ compared to constant value. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:83:3:83:9 | ... == ... | $@ compared to constant value. | test.c:82:10:82:11 | l1 | Function pointer variable l1 |
| test.c:84:3:84:12 | ... == ... | $@ compared to constant value. | test.c:82:10:82:11 | l1 | Function pointer variable l1 |
| test.c:91:3:91:4 | g1 | $@ undergoes implicit constant comparison. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
| test.c:96:7:96:18 | ... == ... | $@ compared to constant value. | test.c:9:9:9:10 | fp | Function pointer variable fp |
| test.c:102:7:102:22 | ... == ... | $@ compared to constant value. | test.c:14:11:14:21 | get_handler | Address of function get_handler |
| test.c:105:7:105:24 | ... == ... | $@ compared to constant value. | test.c:105:7:105:17 | call to get_handler | Expression with function pointer type |
| test.c:121:7:121:13 | ... != ... | $@ compared to constant value. | test.c:3:5:3:6 | f1 | Address of function f1 |
| test.c:133:7:133:13 | ... != ... | $@ compared to constant value. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
| test.c:139:7:139:13 | ... == ... | $@ compared to constant value. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
| test.c:149:8:149:9 | g1 | $@ undergoes implicit constant comparison. | test.c:4:8:4:9 | g1 | Function pointer variable g1 |
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
rules/EXP16-C/DoNotCompareFunctionPointersToConstantValues.ql
153 changes: 153 additions & 0 deletionsc/cert/test/rules/EXP16-C/test.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
#include <stdlib.h>

int f1();
void (*g1)(void);
int (*g2)(int);
void *g3 = NULL;

struct S {
int (*fp)(void);
int x;
};

typedef int (*handler_t)(void);
handler_t get_handler(void);

void f2(void) {
if (f1 == 0) // NON-COMPLIANT
return;

if (f1 > 0) // NON-COMPLIANT
return;

if (f1() == 0) // COMPLIANT
return;

if (f1() > 0) // COMPLIANT
return;

if (g1 == 0) // NON-COMPLIANT
return;

if (g2 == NULL) // NON-COMPLIANT
return;

if (g1 != 0x0) // NON-COMPLIANT
return;

if (f1) // NON-COMPLIANT - implicit comparison
return;

if (g1) // NON-COMPLIANT - implicit comparison
return;
}

void f3(void *p1) {
if (g1 == p1) // COMPLIANT - comparing to variable
return;

if (g2 == g3) // COMPLIANT - comparing to variable
return;
}

void f4(void) {
int (*l1)(void) = 0;

if (f1 == f1) // COMPLIANT - comparing to constant value of same type
return;

if (f1 == l1) // COMPLIANT - comparing to constant value of same type
return;

if (f1 == (int (*)(void))0) // COMPLIANT - explicit cast
return;

if (f1 == (int (*)(void))0) // COMPLIANT - explicit cast
return;

if (f1 == (int (*)(int))0) // NON-COMPLIANT - explicit cast to wrong type
return;

if (f1 == (int)0) // NON-COMPLIANT - cast to non-function pointer type
return;

if (f1 ==
(int)(int (*)(void))
NULL) // NON-COMPLIANT - compliant cast subsumed by non-compliant cast
return;
}

typedef void (*func_t)(void);
void f5(void) {
func_t l1 = g1;
l1 == 0; // NON-COMPLIANT
l1 == NULL; // NON-COMPLIANT
l1 == (func_t)0; // COMPLIANT - cast to function pointer type
}

void f6(void) {
g1 + 0; // COMPLIANT - not a comparison
g1 == g2; // COMPLIANT - not comparing to constant
g1 ? 1 : 0; // NON-COMPLIANT - implicit comparison
}

void f7(void) {
struct S s;
if (s.fp == NULL) // NON-COMPLIANT
f1();

if (s.fp() == NULL) // COMPLIANT
return;

if (get_handler == 0) // NON-COMPLIANT - missing parentheses
return;

if (get_handler() == 0) // NON-COMPLIANT
return;

if (get_handler() == (handler_t)0) // COMPLIANT
return;

if (get_handler()() == 0) // COMPLIANT
return;
}

void f8(void) {
// Test instances of where the function pointer check is used to guard calls
// to that function.

// Technically, this function may perhaps be set to NULL by the linker. But
// it is not a variable that should need to be null-checked at runtime.
if (f1 != 0) // NON-COMPLIANT
{
f1();
}

// Check guards a call, so it is compliant.
if (g1 != 0) // COMPLIANT
{
g1();
}

// Incorrect check, not compliant.
if (g1 != 0) // NON-COMPLIANT
{
f1();
}

// Incorrect check, not compliant.
if (g1 == 0) // NON-COMPLIANT
{
g1();
}

if (g1) // COMPLIANT
{
g1();
}

if (!g1) // NON-COMPLIANT
{
g1();
}
}
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp