Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STR34-C: Rule improvements #577

Open
lcartey opened this issue May 1, 2024 · 0 comments
Open

STR34-C: Rule improvements #577

lcartey opened this issue May 1, 2024 · 0 comments
Assignees
Labels
Difficulty-Low A false positive or false negative report which is expected to take <1 day effort to address false positive/false negative An issue related to observed false positives or false negatives. Impact-High

Comments

@lcartey
Copy link
Collaborator

lcartey commented May 1, 2024

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.
}
@lcartey lcartey added Difficulty-Low A false positive or false negative report which is expected to take <1 day effort to address false positive/false negative An issue related to observed false positives or false negatives. Impact-High labels May 1, 2024
@lcartey lcartey self-assigned this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty-Low A false positive or false negative report which is expected to take <1 day effort to address false positive/false negative An issue related to observed false positives or false negatives. Impact-High
Projects
Development

No branches or pull requests

1 participant