...
In most cases compilers warn about uninitialized variables. These warnings should be resolved as recommended by MSC00-A. Compile cleanly at high warning levels.
...
Non-Compliant Code Example
In this non-compliant code example, the set_flag() function is intended to set a the variable sign to 1 if number is positive and -1 if number is negative. However, the programmer forgot to account for number being 0. If number is 0, then sign remains uninitialized. Because sign is uninitialized, it assumes whatever value is at that location in the program stack. This may lead to unexpected, incorrect program behavior.
| Code Block | ||
|---|---|---|
| ||
void set_flag(int number, int *sign_flag) {
if (sign_flag == NULL) {
return;
}
if (number > 0) {
*sign_flag = 1;
}
else if (number < 0) {
*sign_flag = -1;
}
}
void func(int number) {
int sign;
set_flag(number,&sign);
/* ... */
}
|
Compilers assume that when the address of an uninitialized variable is passed to a function, the variable is initialized within that function. Because compilers frequently fail to diagnose any resulting failure to initialize the variable, the programmer must apply additional scrutiny to ensure the correctness of the code.
Implementation Details
Microsoft Visual Studio 2005, Visual Studio 2008, GCC version 3.4.4, and GCC version 4.1.3 fail to diagnose this error.
Compliant Solution
This defect results from a failure to consider all possible data states (see MSC01-A. Strive for logical completeness). Once the problem is identified, it can be trivially repaired by accounting for the possibility that number can be equal to 0.
| Code Block | ||
|---|---|---|
| ||
void set_flag(int number, int *sign_flag) {
if (sign_flag == NULL) {
return;
}
if (number >= 0) { /* account for number being 0 */
*sign_flag = 1;
} else {
assert( number < 0);
*sign_flag = -1;
}
}
void func(int number) {
int sign;
set_flag(number,&sign);
/* ... */
}
|
Non-Compliant Code Example
| Wiki Markup |
|---|
In this non-compliant code example, the programmer mistakenly fails to set the local variable {{error_log}} to the {{msg}} argument in the {{report_error}} function \[[mercy 06|AA. C References#mercy 06]\]. Because {{error_log}} has not been initialized, it assumes the value already on the stack at this location, which is a pointer to the stack memory allocated to the {{password}} array. The {{sprintf()}} call copies data in {{password}} until a null byte is reached. If the length of the string stored in the {{password}} array is greater than the size of the {{buffer}} array, then a buffer overflow occurs. |
| Code Block | ||
|---|---|---|
| ||
#include <stdio.h>
#include <ctype.h>
#include <string.h>
enum {max_user = 1024};
enum {max_password = 10}; /* sizeof("password\n\0") */
const char* valid_user = "user";
const char* valid_password = "password";
int do_auth(void) {
char username[max_user];
char password[max_password];
puts("Please enter your username: ");
if (fgets(username, sizeof( username), stdin) == NULL) {
/* handle error */
}
/* trim off ws at end, including newline */
while (strlen(username) > 0 &&
isspace( username[ strlen(username) - 1])) {
username[ strlen(username) - 1] = '\0';
}
puts("Please enter your password: ");
if (fgets(password, sizeof( password), stdin) == NULL) {
/* handle error */
}
/* trim off ws at end, including newline */
while (strlen(password) > 0 &&
isspace( password[ strlen(password) - 1])) {
password[ strlen(password) - 1] = '\0';
}
if (!strcmp(username, valid_user) &&
!strcmp(password, valid_password)) {
return 0;
}
return -1;
}
void report_error(char *msg) {
char *error_log;
char buffer[24];
sprintf(buffer, "Error: %s", error_log);
printf("%s\n", buffer);
}
int main(void) {
if (do_auth() == -1) {
report_error("Unable to login");
}
return 0;
}
|
Non-Compliant Code Example
In this non-compliant code example, the report_error function has been modified so that error_log is properly initialized.
| Code Block | ||
|---|---|---|
| ||
void report_error(char *msg) {
char *error_log = msg;
char buffer[24];
sprintf(buffer, "Error: %s", error_log);
printf("%s\n", buffer);
}
|
This solution is still problematic in that a buffer overflow will occur if the the null-terminated byte string referenced by msg is greater than 17 bytes, including the null terminator. The solution also makes use of a "magic number," which should be avoided (see DCL06-A. Use meaningful symbolic constants to represent literal values in program logic).
Compliant Solution
In this solution, the magic number is abstracted and the buffer overflow is eliminated.
| Code Block | ||
|---|---|---|
| ||
enum {max_buffer = 24};
void report_error(char *msg) {
char *error_log = msg;
char buffer[max_buffer];
snprintf(buffer, sizeof( buffer), "Error: %s", error_log);
printf("%s\n", buffer);
}
|
Compliant Solution
A much simpler, less error prone, and better performing compliant solution is shown below.
| Code Block | ||
|---|---|---|
| ||
void report_error(char *msg) {
printf("Error: %s\n", msg);
}
|
...
Risk Assessment
Accessing uninitialized variables generally leads to unexpected program behavior. In some cases these types of flaws may allow the execution of arbitrary code.
...