Page History

...

int privileges;

if (invalid_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 notAccording to the indentation, the programmer may be led to believe that a user is given administrator privileges only when his login is valid.

However, when the programmer adds another statement to the body of the first if statement, the code functions differently.in reality, the else statement actually attaches to the inner if statement, like so:

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 removes the ambiguity and ensures that privileges are correctly assigned.

int privileges;

if (validinvalid_login()) {
  printf("Login Successful\n");  /* debugging line added here */
  if (isallow_normalguests()) {
    privileges = NORMALGUEST;
  } 
} 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

...

\[[ISO/IEC 9899-1999|AA. References#ISO/IEC 9899-1999]\] Section 6.8.4, "Selection statements"
\[[[MISRA 04|AA. References#MISRA 04]\] Rule 14.8
\[[GNU Coding Standards|http://www.gnu.org/prep/standards/standards.html#Syntactic-Conventions]\] Section 5.3, "Clean Use of C Constructs"