I am interested in feedback on a patch I was working on that adds a new attribute (warn_impcast_to_bool) to indicate that the return value of a function shouldn't be used as a boolean, as well as a compile warning and a StaticAnalyzer checker to warn about misusing functions with this attribute. I'd also appreciate any suggestions for how to deal with a class of false positives that the static analysis checker produces.
The change was originally inspired by CVE-2008-5077 , which was the result of an odd design choice in OpenSSL: having an API that returns 1 for success, 0 for failure... and -1 for catastrophic failure. Various API users fell into the trap of treating the return value as a boolean, so the patch adds an attribute to allow this to trigger a warning.
As well as generating a compile-time warning, the patch also includes a new static analyzer checker to catch more indirect uses, where the non-boolean integer value gets propagated via function wrappers or local variables. However, it gives a false positive for cases when the code using the return value actually checks the value in a non-boolean way (because the SVal doesn't reflect the fact that the value has been further constrained). I couldn't see an obvious way to get anything relevant from the RangeConstraintManager; any suggestions?
To test the check (beyond the included unit tests), I annotated dangerous OpenSSL functions and tried building 8 OpenSSL-using codebases with it. So far, this didn't give many results for them: the only possible problem was found in ruby2.1, which was already fixed a few months ago. However, this change is still potentially useful - even 7 years after the original CVE, there are still codebases that fall into OpenSSL's API trap.