Software vulnerabilities can result when a programmer fails to consider all possible data states.
Noncompliant Code Example (if chain)
This noncompliant code example fails to test for conditions where a
is neither b
nor c
. This may be the correct behavior in this case, but failure to account for all the values of a
may result in logic errors if a
unexpectedly assumes a different value.
Code Block | ||
---|---|---|
| ||
if (a == b) { /* ... */ } else if (a == c) { /* ... */ } |
Compliant Solution (if chain)
This compliant solution explicitly checks for the unexpected condition and handles it appropriately.
Code Block | ||
---|---|---|
| ||
if (a == b) { /* ... */ } else if (a == c) { /* ... */ } else { /* handle error condition */ } |
Noncompliant Code Example (switch)
This noncompliant code example fails to consider all possible cases. This may be the correct behavior in this case, but failure to account for all the values of widget_type
may result in logic errors if widget_type
unexpectedly assumes a different value or if its valid range is expanded during code maintenance and the programmer overlooks the need to add a case to the switch.
This is particularly problematic in C, because an identifier declared as an enumeration constant has type int
. As a result, a programmer can accidentally assign an arbitrary integer value to an enum
type, as shown in this example.
Code Block | ||
---|---|---|
| ||
enum WidgetEnum { WE_W, WE_X, WE_Y, WE_Z } widget_type; widget_type = 45; switch (widget_type) { case WE_X: /* ... */ break; case WE_Y: /* ... */ break; case WE_Z: /* ... */ break; } |
Implementation Details
Microsoft Visual C++ .NET with /W4
does not warn when assigning an integer value to an enum
type, or when the switch statement does not contain all possible values of the enumeration.
Compliant Solution (switch)
This compliant solution explicitly checks for the unexpected condition by adding a default
clause to the switch statement.
Code Block | ||
---|---|---|
| ||
enum WidgetEnum { WE_W, WE_X, WE_Y, WE_Z } widget_type; widget_type = WE_X; switch (widget_type) { case WE_W: /* ... */ break; case WE_X: /* ... */ break; case WE_Y: /* ... */ break; case WE_Z: /* ... */ break; default: /* can't happen */ /* handle error condition */ break; } |
Adding a default case to a switch statement, even when all possible switch labels are specified, is an allowable exception (MSC07-EX1) to MSC07-C. Detect and remove dead code, as the unreachable code is added as a precautionary measure.
Historical Discussion
This practice has been a subject of debate for some time, but a clear direction has emerged.
Originally, the consensus among those writing best practices was simply that each switch
statement should have a default
label. Eventually there emerged compilers and static analysis tools that could verify that a switch
on an enum
type contained a case
label for each enumeration value, but only if no default
label existed. This led to a shift toward purposely leaving out the default
label to allow static analysis. However, the resulting code was then vulnerable to enum
variables being assigned int
values outside the set of enum
values.
These two practices have now been merged. A switch
on an enum
type should now contain a case
label for each enum
value, but should also contain a default
label for safety. This is not more difficult to analyze statically.
Existing implementations are in transition, with some not yet analyzing switch
statements with default
labels. Developers must take extra care to check their own switch
statements until the new practice becomes universal.
Noncompliant Code Example (Zune 30)
This noncompliant code example shows incomplete logic when converting dates. The code appeared in the Zune 30 media player, causing many players to lock up on December 30, 2008, at midnight PST. This noncompliant code example comes from the ConvertDays
function in the real-time clock (RTC) routines for the MC13783 PMIC RTC. This noncompliant code sample takes the number of days since January 1, 1980, and computes the correct year and number of days since January 1 of the correct year.
The flaw in the code occurs when days
has the value 366, because the loop never terminates. This bug manifested itself on the 366th day of 2008, which was the first leap year in which this code was active.
Code Block | ||
---|---|---|
| ||
#define ORIGINYEAR 1980 UINT32 days = /* number of days since January 1, 1980 */ int year = ORIGINYEAR; /* ... */ while (days > 365) { if (IsLeapYear(year)) { if (days > 366) { days -= 366; year += 1; } } else { days -= 365; year += 1; } } |
Compliant Solution (Zune 30)
This proposed rewrite is provided by http://www.aeroxp.org/2009/01/lesson-on-infinite-loops. The loop is guaranteed to exit, as days
decreases for each iteration of the loop, unless the while
condition fails, and the loop terminates.
Code Block | ||
---|---|---|
| ||
#define ORIGINYEAR 1980 UINT32 days = /* input parameter */ int year = ORIGINYEAR; /* ... */ int daysThisYear = (IsLeapYear(year) ? 366 : 365); while (days > daysThisYear) { days -= daysThisYear; year += 1; daysThisYear = (IsLeapYear(year) ? 366 : 365); } |
This compliant solution is for illustrative purposes and is not necessarily the solution implemented by Microsoft.
Risk Assessment
Failing to take into account all possibilities within a logic statement can lead to a corrupted running state, potentially resulting in unintentional information disclosure or abnormal termination.
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
MSC01-C | medium | probable | medium | P8 | L2 |
Automated Detection
The LDRA tool suite Version 7.6.0 can detect violations of this recommendation.
GCC Compiler Version can detect some violations of this recommendation when the -Wswitch
and -Wswitch-default
flags are used.
Compass/ROSE can detect some violations of this recommendation. In particular it flags switch statements that do not have a default clause. ROSE should also detect 'fake switches' as well...that is a chain of if
statements each checking the value of the same variable. These if statements should always end in an 'else' clause, or they should mathematically cover every possibility. For instance:
Code Block | ||
---|---|---|
| ||
if (x > 0) { /* ... */ } else if (x < 0) { /* ... */ } else if (x == 0) { /* ... */ } |
Klocwork Version 8.0.4.16 can detect violations of this rule with the LA_UNUSED checker.
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Other Languages
This rule appears in the C++ Secure Coding Standard as MSC01-CPP. Strive for logical completeness.
References
Wiki Markup |
---|
\[[Hatton 95|AA. References#Hatton 95]\] Section 2.7.2, "Errors of omission and addition" \[[ISO/IEC PDTR 24772|AA. References#ISO/IEC PDTR 24772]\] "CLL Switch statements and static analysis" \[[Viega 05|AA. References#Viega 05]\] Section 5.2.17, "Failure to account for default case in switch" \[[http://www.aeroxp.org/2009/01/lesson-on-infinite-loops]\] for analysis on the Zune 30 bug |