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.
In the event that either of these statements are used in a macro, this implies that 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 therefore 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 probably misleading) guide to the structure.
Noncompliant Code Example
This noncompliant code example uses an if-else
statement without braces to authenticate a user.
int login; if (invalid_login()) login = 0; else login = 1;
The programmer adds a debugging statement to determine when the login is valid, but forgets 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 is not functioning 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-else
statement nested in another if
statement, always put braces around the if-else
.
This noncompliant code example does not use braces.
int privileges; if (valid_login()) if (is_normal()) privileges = NORMAL; 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.
int privileges; if (valid_login()) printf("Login Successful\n"); /* debugging line added here */ if (is_normal()) /* this if-else clause is always evaluated even if the user is not logged in! */ privileges = NORMAL; 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 credentials.
Compliant Solution
Adding braces ensures that privileges are correctly assigned.
int privileges; if (valid_login()) { printf("Login Successful\n"); /* debugging line added here */ if (is_normal()) { privileges = NORMAL; } else { privileges = ADMINISTRATOR; } }
Noncompliant Code Example
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.
#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:
int x, y, z; if (z == 0) DEC(x, y);
This will expand to:
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
Given an if
statement bounded with opening and closing braces, the macro would expand as intended.
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
[[ISO/IEC 9899-1999]] Section 6.8.4, "Selection statements"
[GNU Coding Standards] Section 5.3, "Clean Use of C Constructs"