`STR34-C`: Rule improvements
Open
@lcartey is already working on this.
Since May 1, 2024.
Assessment
This issue has not been assessed yet.
Description
Difficulty-Medium
false positive/false negative
Impact-High
Standard-CERT-C
Affected rules
STR34-C
Description
- Do not consider specifiers when considering whether a type is a
chartype - whether a type isconst,volatileetc. doesn't impact whether it's vulnerable to this bug. - Exclude cases where the range of the casted value does not contain negative values - this is because only negative signed
charvalues are modified by the conversion to a larger signed integer. - Do not consider conversions to larger unsigned integers (they are excluded by the rule).
- Do not report issues on platforms on which
charis unsigned by default. Currently we say we wantCharTypes but notUnsignedCharTypes, however that does not exclude the case wherecharis unsigned. I think we want the equivalent ofc.getExpr().getType().(CharType).isSigned()(notwithstanding the first point in the list about specifiers) - Ignore implicit integer promotion conversions which occur as part of an equality or inequality comparison, where the other side of the comparison is also a signed char. In this specific case, the equality only holds if it would have held before the conversions.
- We could also consider excluding the common pattern of
(a >= 'A' && a <= ' F')and similar. These are safe as long as the two constants are within the range[0..CHAR_MAX]. - We should also consider how to handle calls to library macros (such as
tolower) which often create multiple results, which can be confusing to the user.
Example
void example_function(const char x) {
if (x == EOF) ; // NON_COMPLIANT[FALSE_NEGATIVE] - missed because `x` is a `const char`
if ('1' == EOF) ; // COMPLIANT[FALSE_POSITIVE] - assuming ASCII `1` can be represented by larger signed integral types, this is not a problem
if (x == 1u) ; // Excluded from the rule by definition
if (x == '~') ; // COMPLIANT - comparison valid - both sides have the same conversion applied
if (x > '~') ; // NON_COMPLIANT - comparison isn't valid - `x` may be negative.
}
- Dominant language
- CodeQL
- Stars
- 227
- Forks
- 82
- Avg merge
- 6d 7h
- Merged PRs (30d)
- 9
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/codeql-coding-standards
-
false positive/false negative Stardard-MISRA-C++
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
github/codeql-coding-standards#1172 ·
-
Difficulty-Low false positive/false negative false-negative Impact-Low Standard-MISRA-C
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty-Medium false positive/false negative false-positive Impact-Medium Standard-CERT-C
Difficulty 4/5 3-5 days Newbie friendliness 48/100
github/codeql-coding-standards#1200 ·
-
`RULE-0-0-1`: "unreachable statement" false positives due to over-pruning of the control-flow graph Openfalse positive/false negative
Difficulty 4/5 3-5 days Newbie friendliness 48/100
github/codeql-coding-standards#1190 ·
-
false positive/false negative
Difficulty 3/5 1-2 days Newbie friendliness 65/100
github/codeql-coding-standards#1175 ·