Skip to content

STR34-C: Rule improvements #577

Open
@lcartey

Description

@lcartey

Affected rules

  • STR34-C

Description

  • Do not consider specifiers when considering whether a type is a char type - whether a type is const, 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 want CharTypes but not UnsignedCharTypes, however that does not exclude the case where char is unsigned. I think we want the equivalent of c.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.
}

Metadata

Metadata

Assignees

Labels

Difficulty-MediumA false positive or false negative report which is expected to take 1-5 days effort to addressImpact-HighStandard-CERT-Cfalse positive/false negativeAn issue related to observed false positives or false negatives.

Type

No type

Projects

Status

Triaged

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions