[C-safe-secure-studygroup] "no unused <blah>" rules
Martin Sebor
msebor at gmail.com
Wed Mar 15 17:47:37 UTC 2017
On 03/15/2017 11:25 AM, Aaron Ballman wrote:
> On Wed, Mar 15, 2017 at 1:12 PM, Martin Sebor <msebor at gmail.com> wrote:
>> On 03/14/2017 09:22 AM, Aaron Ballman wrote:
>>>
>>> I was asked to look into Rule 2.2 and the related "no unused <blah>"
>>> rules (2.3, 2.4, 2.5, 2.6, 2.7) to see if we should make a holistic
>>> decision about whether to include them in a security and/or safety
>>> profile.
>>>
>>> TL;DR: safety profile makes sense, security profile does not appear to
>>> make sense.
>>
>>
>> I agree with this assessment for 2.3 and above.
>>
>> For 2.2 Dead Code (reachable statements with no effect, as I
>> understand the rule), unlike unused declarations, such statements
>> often are signs of a bug. This is discussed in detail in CERT
>> Secure Standard rule MSC12-C. As a result the security profile
>> would benefit from such a rule. I suspect the reason MSC12-C is
>> not included in TS 17961 is because it's not decidable (even
>> though other undecidable rules are included).
>
> To be clear, MSC12-C is a CERT recommendation rather than a rule. I'm
> not certain decidability was a factor in that decision so much as the
> number of false positives from perfectly sensible dead code. Consider:
>
> const char *Ptr = StartPtr + offsetof(some_struct, some_field);
>
> This code has dead code if some_field is the first field in a
> structure (because StartPtr + 0 has no effect on the code).
The rule would likely need to be specified so as not to require
obvious false positives. A formulation that would avoid this
case is to have it apply to complete simple statements.
> Other such
> examples exist (calls to assert when NDEBUG is defined, etc) as well.
I'm sure they do, and like all undecidable rules, diagnosing
dead code would unavoidably be prone to some false positives
(and/or false negatives).
Regarding assert when NDBUG is defined, those expand to nothing
so they're not dead code, are they? (Though asserts involving
expressions with side effects are a class of bugs that I think
should be diagnosed in all profiles).
> We may still want to consider something like 2.2, but it feels a lot
> more like a stylistic question than a security question. Is having
> dead code a bad practice? For egregious cases of it, absolutely! Is
> having dead code likely to lead to a security vulnerability? I'm not
> aware of any instance of it that resulted in a CVE (but I could
> certainly be wrong).
Apple's Goto Fail bug is an example of a vulnerability caused
by dead code due to a spurious goto statement: CVE-2014-1266.
Martin
>
> ~Aaron
>
>>
>> Other than that, I also agree with the points you make below
>> about the specification of the rules needing to be tightened
>> up to use C standard terminology and avoid potential ambiguity.
>>
>> Martin
>>
>>>
>>> There appears to be no security implications for this class of rules.
>>> Something that is unused (or is dead code), by definition, has no
>>> effect on the semantics of the application, and therefore has no
>>> security impact. While it does indicate a bad code "smell", the
>>> presence of dead or unused constructs by itself does not indicate a
>>> possible security vulnerability, so I think it should be excluded from
>>> a security profile.
>>>
>>> By the same logic, there's not really a safety implication either
>>> since the code has no impact on the execution of the application.
>>> However, there are other implications in safety than just impact to
>>> execution. For instance, there is a question about product liabilities
>>> involving injury or death -- the presence of dead code made be used as
>>> an indicator of quality by an expert witness [0]. This suggests there
>>> are valid reasons to include this rule in a safety profile, even if
>>> those reasons are not strictly in the programming domain.
>>>
>>> If we decide to include these rules in a safety profile, we will need
>>> to tighten up their definitions. For instance, all of the rules
>>> presume that code in the program exists because the programmer was the
>>> one providing the code rather than a 3rd party library providing the
>>> code. e.g., a library providing a typedef or a tag that is unused by
>>> the application would indicate a violation of Rule 2.3 or Rule 2.4. It
>>> seems overly constraining to expect conforming code to modify 3rd
>>> party libraries to remove unused code.
>>>
>>> Specific to Rule 2.5, there is no clear definition of what constitutes
>>> the use of a macro. e.g., it is unclear whether this code uses the
>>> NDEBUG macro or not, as it depends on whether BE_SURE_THINGS_WORK is
>>> defined. Does the macro need to be used in all configurations of the
>>> program, or merely some of them?
>>>
>>> #define NDEBUG
>>> // ...
>>> #if defined(BE_SURE_THINGS_WORK)
>>> assert(a == 12 && "a isn't 12 and that's bad");
>>> #endif
>>>
>>> Specific to Rule 2.6, the rule claims to be decidable and checkable in
>>> a single translation unit, but I'm not certain that agrees with the
>>> definition of a label. Consider the labels in the following code:
>>>
>>> enum e {
>>> one,
>>> two
>>> };
>>>
>>> void foo(enum e val) {
>>> switch (val) {
>>> case one: do_something(); break;
>>> case two: do_something_else(); break;
>>> default: assert(false && "Say what?");
>>> }
>>>
>>> In this code, default is a label (see C11 6.8.1), but deciding whether
>>> it's used or unused requires analysis of the callers of foo(). It's
>>> debatable whether this rule should apply to case or default labels at
>>> all.
>>>
>>> ~Aaron
>>>
>>> [0]
>>> http://embeddedgurus.com/barr-code/2013/02/dead-code-the-law-and-unintended-consequences/
>>>
>>> _______________________________________________
>>> C-safe-secure-studygroup mailing list
>>> C-safe-secure-studygroup at lists.trustable.io
>>>
>>> https://lists.trustable.io/cgi-bin/mailman/listinfo/c-safe-secure-studygroup
>>>
>>
>>
>> _______________________________________________
>> C-safe-secure-studygroup mailing list
>> C-safe-secure-studygroup at lists.trustable.io
>> https://lists.trustable.io/cgi-bin/mailman/listinfo/c-safe-secure-studygroup
>
> _______________________________________________
> C-safe-secure-studygroup mailing list
> C-safe-secure-studygroup at lists.trustable.io
> https://lists.trustable.io/cgi-bin/mailman/listinfo/c-safe-secure-studygroup
>
More information about the C-safe-secure-studygroup
mailing list