...
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 nullNULL 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") */ char const char* valid_user = "user"; char 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 const char *msg) { char const 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; } |
...
Code Block | ||
---|---|---|
| ||
void report_error(char const char *msg) { char const 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 nullNULL-terminated byte string referenced by msg
is greater than 17 bytes, including the null 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).
...
Code Block | ||
---|---|---|
| ||
enum {max_buffer = 24}; void report_error(char const char *msg) { char const char *error_log = msg; char buffer[max_buffer]; snprintf(buffer, sizeof( buffer), "Error: %s", error_log); printf("%s\n", buffer); } |
...
Code Block | ||
---|---|---|
| ||
void report_error(char const char *msg) { printf("Error: %s\n", msg); } |
...
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
EXP33-C | 3 ( high ) | 1 ( unlikely ) | 2 ( medium ) | P6 | L2 |
Automated Detection
The LDRA tool suite V 7.6.0 is able to detect violations of this rule.
...
EXP32-C. Do not cast away a volatile qualification 03. Expressions (EXP) EXP34-C. Ensure a null NULL pointer is not dereferenced