|
| 1 | +# EXP16-C: Do not compare function pointers to constant values |
| 2 | + |
| 3 | +This query implements the CERT-C rule EXP16-C: |
| 4 | + |
| 5 | +> Do not compare function pointers to constant values |
| 6 | +
|
| 7 | + |
| 8 | +## Description |
| 9 | + |
| 10 | +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. |
| 11 | + |
| 12 | +## Noncompliant Code Example |
| 13 | + |
| 14 | +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. |
| 15 | + |
| 16 | +```cpp |
| 17 | +/* First the options that are allowed only for root */ |
| 18 | +if (getuid == 0 || geteuid != 0) { |
| 19 | + /* ... */ |
| 20 | +} |
| 21 | + |
| 22 | +``` |
| 23 | + |
| 24 | +## Noncompliant Code Example |
| 25 | + |
| 26 | +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). |
| 27 | + |
| 28 | +```cpp |
| 29 | +/* First the options that are allowed only for root */ |
| 30 | +if (getuid() == 0 || geteuid != 0) { |
| 31 | + /* ... */ |
| 32 | +} |
| 33 | + |
| 34 | +``` |
| 35 | + |
| 36 | +## Compliant Solution |
| 37 | + |
| 38 | +The solution is to provide the open and close parentheses following the `geteuid` token so that the function is properly invoked: |
| 39 | + |
| 40 | +```cpp |
| 41 | +/* First the options that are allowed only for root */ |
| 42 | +if (getuid() == 0 || geteuid() != 0) { |
| 43 | + /* ... */ |
| 44 | +} |
| 45 | + |
| 46 | +``` |
| 47 | + |
| 48 | +## Compliant Solution |
| 49 | + |
| 50 | +A function pointer can be compared to a null function pointer of the same type: |
| 51 | + |
| 52 | +```cpp |
| 53 | +/* First the options that are allowed only for root */ |
| 54 | +if (getuid == (uid_t(*)(void))0 || geteuid != (uid_t(*)(void))0) { |
| 55 | + /* ... */ |
| 56 | +} |
| 57 | + |
| 58 | +``` |
| 59 | +This code should not be diagnosed by an analyzer. |
| 60 | + |
| 61 | +## Noncompliant Code Example |
| 62 | + |
| 63 | +In this noncompliant code example, the function pointer `do_xyz` is implicitly compared unequal to 0: |
| 64 | + |
| 65 | +```cpp |
| 66 | +int do_xyz(void); |
| 67 | + |
| 68 | +int f(void) { |
| 69 | +/* ... */ |
| 70 | + if (do_xyz) { |
| 71 | + return -1; /* Indicate failure */ |
| 72 | + } |
| 73 | +/* ... */ |
| 74 | + return 0; |
| 75 | +} |
| 76 | + |
| 77 | +``` |
| 78 | +
|
| 79 | +## Compliant Solution |
| 80 | +
|
| 81 | +In this compliant solution, the function `do_xyz()` is invoked and the return value is compared to 0: |
| 82 | +
|
| 83 | +```cpp |
| 84 | +int do_xyz(void); |
| 85 | + |
| 86 | +int f(void) { |
| 87 | +/* ... */ |
| 88 | + if (do_xyz()) { |
| 89 | + return -1; /* Indicate failure */ |
| 90 | + } |
| 91 | +/* ... */ |
| 92 | + return 0; |
| 93 | +} |
| 94 | +
|
| 95 | +``` |
| 96 | + |
| 97 | +## Risk Assessment |
| 98 | + |
| 99 | +Errors of omission can result in unintended program flow. |
| 100 | + |
| 101 | +<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> |
| 102 | + |
| 103 | + |
| 104 | +## Automated Detection |
| 105 | + |
| 106 | +<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> |
| 107 | + |
| 108 | + |
| 109 | +## Related Vulnerabilities |
| 110 | + |
| 111 | +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). |
| 112 | + |
| 113 | +## Related Guidelines |
| 114 | + |
| 115 | +<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> |
| 116 | + |
| 117 | + |
| 118 | +## Bibliography |
| 119 | + |
| 120 | +<table> <tbody> <tr> <td> \[ <a> Hatton 1995 </a> \] </td> <td> Section 2.7.2, "Errors of Omission and Addition" </td> </tr> </tbody> </table> |
| 121 | + |
| 122 | + |
| 123 | +## Implementation notes |
| 124 | + |
| 125 | +None |
| 126 | + |
| 127 | +## References |
| 128 | + |
| 129 | +* CERT-C: [EXP16-C: Do not compare function pointers to constant values](https://wiki.sei.cmu.edu/confluence/display/c) |
0 commit comments