Incorrect use of _When_ annotation with non-pointer values #608
Description
NOTE: This has been copied from MSFT bug 59941779
This issue concerns several templated functions within the file: /onecore/shell/published/inc/wil/opensource/wil/result_macros.h
An example can be found below:
template <__R_CONDITIONAL_PARTIAL_TEMPLATE typename PointerT, __R_ENABLE_IF_IS_CLASS(PointerT)>
__WI_SUPPRESS_NULLPTR_ANALYSIS _When_(pointer == nullptr, _Analysis_noreturn_)
__R_CONDITIONAL_NOINLINE_TEMPLATE_METHOD(void, Throw_GetLastErrorIfNullMsg)
(__R_CONDITIONAL_FN_PARAMS _In_opt_ const PointerT& pointer, _Printf_format_string_ PCSTR formatString, ...)
{
if (pointer == nullptr)
{This template is valid only when the value supplied by PointerT is a class, in other words not a pointer (pointers are handled in a different variant declared above this one).
In the SAL annotation _When_(pointer == nullptr, _Analysis_noreturn_), PREfast is unable to resolve the expression pointer == nullptr because the value is not a pointer itself. The compiler itself is able to resolve it within the function body due to various overloaded functions applied to the classes the funtion is used with. However the SAL expression cannot be evaluated in the same way due to technical limitations.
Unfortunately the side effect is that a failure to evaluate this expression causes the PREfast checker to assume that _Analysis_noreturn_ to be considered as always valid, and this can have further effects such as:
- False positives on rule C29840 (Unreachable code)
- Failures to detect other security issues (such as buffer overflows)
I've noted also that __WI_SUPPRESS_NULLPTR_ANALYSIS causes the suppression of another rule C28285 which actually points out this flaw in the _When_ annotation.
To fix this issue, the solution would be to:
- Remove the annotation
_When_(pointer == nullptr, _Analysis_noreturn_) - Also remove the
__WI_SUPPRESS_NULLPTR_ANALYSISsuppression preceding it
The following list shows the functions where the issue was observed:
Throw_IfNullAllocMsgThrow_HrIfNullMsgThrow_GetLastErrorIfNullMsg