You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 37 Next »

Software vulnerabilities can result when a programmer fails to consider all possible data states.

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.

if (a == b) {
  /* ... */
}
else if (a == c) {
  /* ... */
}

Compliant Solution

This compliant solution explicitly checks for the unexpected condition and handles it appropriately.

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 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.

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.

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-A. 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.

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-A

medium

probable

medium

P8

L2

Automated Detection

The LDRA tool suite V 7.6.0 is able to detect violations of this recommendation.

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:

  if (x > 0) {
	  /* ... */
  } else if (x < 0) {
    /* ... */
  } else if (x == 0) {
    /* ... */
  }

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

[[Hatton 95]] Section 2.7.2, "Errors of omission and addition"
[[ISO/IEC PDTR 24772]] "CLL Switch statements and static analysis"
[[Viega 05]] Section 5.2.17, "Failure to account for default case in switch"


MSC00-A. Compile cleanly at high warning levels      13. Miscellaneous (MSC)       MSC02-A. Avoid errors of omission

  • No labels