Opening and closing braces for if
, for
, or while
statements should always be used, even if said statement's body contains only a single statement.
If an if
, while
, or for
statement is used in a macro, then the macro definition should not be concluded with a semicolon (see PRE11-C. Do not conclude macro definitions with a semicolon).
Braces help improve the uniformity and readability of code.
More importantly, when inserting an additional statement in a body containing only a single statement, it is easy to forget to add braces when the indentation tends to give a strong (but misleading) guide to the structure.
Braces also help ensure that macros with multiple statements are properly expanded. Such a macro should be wrapped in a do-while loop (see PRE10-C. Wrap multi-statement macros in a do-while loop); however, when the do-while loop is not present, braces can still ensure that the macro expands as intended.
Noncompliant Code Example
This noncompliant code example uses an if
statement without braces to authenticate a user.
int login; if (invalid_login()) login = 0; else login = 1;
A developer might add a debugging statement to determine when the login is valid, but forget to add opening and closing braces.
int login; if (invalid_login()) login = 0; else printf("Login is valid\n"); /* debugging line added here */ login = 1; /* this line always gets executed, regardless of a valid login! */
Due to the indentation of the code, it is difficult to tell that the code will not function as intended by the programmer, leading to a possible security breach.
Compliant Solution
Opening and closing braces are used even when the body is a single statement.
int login; if (invalid_login()) { login = 0; } else { login = 1; }
Noncompliant Code Example
When you have an if
statement nested in another if
statement, always put braces around if
and else
bodies.
This noncompliant code example does not use braces.
int privileges; if (invalid_login()) if (allow_guests()) privileges = GUEST; else privileges = ADMINISTRATOR;
According to the indentation, the programmer may be led to believe that a user is given administrator privileges only when his login is valid.
However, in reality, the else
statement actually attaches to the inner if
statement, like so:
int privileges; if (invalid_login()) if (allow_guests()) privileges = GUEST; else privileges = ADMINISTRATOR;
This is a security loophole - users with invalid logins can still obtain administrator privileges.
Compliant Solution
Adding braces removes the ambiguity and ensures that privileges are correctly assigned.
int privileges; if (invalid_login()) { if (allow_guests()) { privileges = GUEST; } } else { privileges = ADMINISTRATOR; }
Risk Assessment
Recommendation |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
EXP19-C |
medium |
probable |
medium |
P8 |
L2 |
References
[ISO/IEC 9899-1999] Section 6.8.4, "Selection statements"
[MISRA 04] Rule 14.8
[GNU Coding Standards] Section 5.3, "Clean Use of C Constructs"
EXP18-C. Do not perform assignments in conditional expressions 03. Expressions (EXP) EXP20-C. Perform explicit tests to determine success, true-false, and equality