Software vulnerabilities can result when a programmer fails to consider all possible data states.
...
"may result in logic errors if widget_type unexpectedly assumes a different value" should have appended "or if its valid range is expanded during code maintenance and the programmer overlooks the need to add a case to the switch".
Is adding a default case really an instance of "remove dead code"? It seems like more of an exception, where unreachable code is added as a precautionary measure.
I think a useful common practice should be shown by using
default: /* "can't happen" */
which shows that it handles an internal logic error.
Non-Compliant Code Example
This 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
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 */
}
|
Non-Compliant Code Example
This non-compliant 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. This is particularly problematic in C, because an identifier declared as an enumeration constant has type int
. Therefore, 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
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:
/* handle error condition */
break;
}
|
Adding a default case to a switch statement even when all possible switch labels are specified is an instance of exception MSC07-EX1 to MSC07-A. Detect and remove dead code.
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 no 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.
...
Risk Assessment
Failing to take into account all possibilities within a logic statement can lead to a corrupted running state, possibly resulting in unintentional information disclosure or abnormal termination.
...