Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Edited by sciSpider (sch jbop) (X_X)@==(Q_Q)@

...

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
bgColor#FFCCCC
#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
bgColor#ffcccc
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
bgColor#ccccff
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
bgColor#ccccff
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