Opening and closing braces for if
, for
, or and while
statements should always be used , even if said the statement's body contains only a single statement.
In the event that either of these statements are If an if
, while
, or for
statement is used in a macro, this implies that the macro definition should not be concluded conclude with a semicolon. (see See PRE11-C. Do not conclude macro definitions with a semicolon.).
Braces help improve the uniformity , and therefore readability of code. More importantlyimportant, when inserting an additional statement in into a body containing only a single statement, it is easy to forget to add braces when because the indentation tends to give a gives strong (but probably misleading) guide guidance 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 multistatement 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-else
statement without braces to authenticate a user.:
Code Block | ||||
---|---|---|---|---|
| ||||
int login;
if (invalid_login())
login = 0;
else
login = 1;
|
The programmer adds A developer might add a debugging statement to determine when the login is valid , but forgets forget to add opening and closing braces.:
Code Block | ||||
---|---|---|---|---|
| ||||
int login; if (invalid_login()) login = 0; else printf("Login is valid\n"); /* debuggingDebugging line added here */ login = 1; /* thisThis line always gets executed, /* regardless of a valid login! */ |
Due to the Because of the indentation of the code, it is difficult to tell that the code is will not functioning function as intended by the programmer, potentially leading to a possible security breach.
Compliant Solution
Opening In the compliant solution, opening and closing braces are used even when the body is a single statement.:
Code Block | ||||
---|---|---|---|---|
| ||||
int login;
if (invalid_login()) {
login = 0;
} else {
login = 1;
}
|
Noncompliant Code Example
When you have This noncompliant code example has an if-else
statement nested in another if
statement , always put without braces around the if-else
. This noncompliant code example does not use braces. and else
bodies:
Code Block | ||||
---|---|---|---|---|
| ||||
int privileges; if (validinvalid_login()) if (isallow_normalguests()) privileges = NORMALGUEST; else privileges = ADMINISTRATOR; |
It works as expected by setting the privileges
variable accordingly, depending on whether the logged in user is an administrator or not.
However, when the programmer adds another statement to the body of the first if
statement, the code functions differently.The indentation could lead the programmer to believe that a user is given administrator privileges only when the user's login is valid. However, the else
statement actually attaches to the inner if
statement:
Code Block | ||||
---|---|---|---|---|
| ||||
int privileges; if (validinvalid_login()) printf("Login Successful\n"); /* debugging line added here */ if (isallow_normalguests()) /* this if-else clause is always evaluated even if the user is not logged in! */ privileges = NORMALGUEST; else privileges = ADMINISTRATOR; |
Because of the additional statement in the body of the first if
statement, the user can easily gain administrator privileges, without having to even provide valid login credentialsThis is a security loophole: users with invalid logins can still obtain administrator privileges.
Compliant Solution
Adding braces In the compliant solution, adding braces removes the ambiguity and ensures that privileges are correctly assigned.:
Code Block | ||||
---|---|---|---|---|
| ||||
int privileges; if (validinvalid_login()) { printf("Login Successful\n"); /* debugging line added here */ if (isallow_normalguests()) { privileges = NORMALGUEST; } } else { privileges = ADMINISTRATOR; } } |
Noncompliant Code Example
...
(empty block)
This noncompliant code example has a while
statement with no block:
Macros can be used to execute a sequence of multiple statements as group.
Note that a macro with multiple statements should be wrapped in a do-while loop (see PRE10-C. Wrap multi-statement macros in a do-while loop), but for the purposes of this example it is not. However, the situation can still be salvaged if braces are used in the if
statement.
Code Block | ||
---|---|---|
| ||
#define DEC(x,y) \
printf("Initial value was %d\n", x); \
x -= y; \
printf("Current value is %d\n", x)
|
This macro will expand correctly in a normal sequence of statements, but not as the then-clause in an if
statement:
Code Block | ||
---|---|---|
| ||
int x, y, z;
if (z == 0)
DEC(x, y);
|
This will expand to:
Code Block | ||
---|---|---|
| ||
int x, y, z;
if (z == 0)
printf("Initial value was %d\n", x);
x -= y;
printf("Current value is %d\n", x);
|
Compliant Solution
| |||
while (invalid_login()); |
Note that if invalid_login()
has no side effects (such as warning the user if their login failed), this code also violates MSC12-C. Detect and remove code that has no effect or is never executed.
Compliant Solution (empty block)
This compliant solution features an explicit empty block, which clarifies the developer's intent:Given an if
statement bounded with opening and closing braces, the macro would expand as intended.
Code Block | ||||
---|---|---|---|---|
| ||||
while (invalid_login()) { int x, y, z; if (z == 0) { printf("Initial value was %d\n", x); x -= y; printf("Current value is %d\n", x) } |
Risk Assessment
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
EXP19-C |
Medium |
Probable |
Medium | P8 | L2 |
References
Automated Detection
Tool | Version | Checker | Description | ||||||
Astrée |
| compound-ifelse compound-loop | Fully checked | ||||||
Axivion Bauhaus Suite |
| CertC-EXP19 | Fully implemented | ||||||
Helix QAC |
| C2212 | |||||||
Klocwork |
| MISRA.IF.NO_COMPOUND MISRA.STMT.NO_COMPOUND | |||||||
LDRA tool suite |
| 11 S, 12 S, 428 S | Fully Implemented | ||||||
Parasoft C/C++test |
| CERT_C-EXP19-a | The statement forming the body of a 'switch', 'while', 'do...while' or 'for' statement shall be a compound statement | ||||||
PC-lint Plus |
| 9012 | Fully supported | ||||||
Polyspace Bug Finder |
| CERT C: Rec. EXP19-C | Checks for iteration or selection statement body not enclosed in braces (rec. fully covered) | ||||||
PVS-Studio |
| V563, V628, V640, V705 | |||||||
RuleChecker |
| compound-ifelse compound-loop | Fully checked | ||||||
SonarQube C/C++ Plugin |
| S121 |
Related Vulnerabilities
CVE-2014-1266 was due, in large part, to failing to follow this recommendation. There is a spurious "goto fail" statement on line 631 of sslKeyExchange.c. This "goto" gets executed unconditionally, even though it is indented as if it were part of the preceding "if" statement. As a result, the call to sslRawVerify (which performs the actual signature verification) is rendered dead code. [ImperialViolet 2014]. If the body of the "if" statement had been enclosed in braces, then this defect likely would not have happened.
Related Guidelines
MISRA C:2012 | Rule 15.6 (required) |
Bibliography
...
\[[ISO/IEC 9899-1999|AA. References#ISO/IEC 9899-1999]\] Section 6.8.4, "Selection statements"
\[GNU Coding Standards|http://www.gnu.org/prep/standards/standards.html#Syntactic-Conventions\] Section 5.3, "Clean Use of C Constructs" Wiki Markup