codeql-coding-standards
codeql-coding-standards copied to clipboard
`STR34-C`: Rule improvements
Affected rules
-
STR34-C
Description
- Do not consider specifiers when considering whether a type is a
char
type - whether a type isconst
,volatile
etc. 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
char
values 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
char
is unsigned by default. Currently we say we wantCharType
s but notUnsignedCharType
s, however that does not exclude the case wherechar
is 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.
}