Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Eliminated strcpy_s, no change in string handling between examples now

...

Reusing variable names leads to programmer confusion about which variable is being modified. Additionally, if variable names are reused, generally one or both of the variable names are too generic.

Non-Compliant Code Example

...

In this non-compliant code example, the programmer sets the value of the msg variable, expecting to reuse it outside the block. Due to the reuse of the variable name, however, the outside msg variable value is not changed.

Code Block
bgColor#FFCCCC
char msg[100];
/* ... */
void error_message(char *error_msg) {
  char msg[80];
  /* ... */
  strcpy(msg, error_msg);  /* Assume error_msg isisn't assumed to reference a NTBS of len 99 or less too long */
  return;
}

Furthermore, if the length of the null-terminated byte string referenced by error_msg is greater than 79 characters in length, a buffer overflow will occur on the stack, which may be exploitable.

...

strcpy

...

Wiki Markup
In this non-compliant code example, the call to {{strpcy()}} has been replaced with a call to {{strcpy_s()}}.  See \[[STR00-A. Use TR 24731 for remediation of existing string manipulation code]\] for more information on using {{strcpy_s()}}.

Code Block
bgColor#FFCCCC

char msg[100];
/* ... */
void error_message(char *error_msg) {
  char msg[80];
  (msg, error_msg);
  return;
}
/* ... */

  /* Ensure error_msg isisn't assumed to reference a NTBS of length 99 or less too long */
  errno_t e = strcpy_s(msg, sizeof(msg),if (strlen( error_msg);
 >= if sizeof(e != 0 msg)) {
     /* handle strcpy_s() error */
  }
}
error_msg[sizeof(msg) - 1] = '\0';
}
error_message( error_msg); /* oops! */

Furthermore, if the length of the null-terminated byte string referenced by error_msg is greater than 79 characters in length, a buffer overflow will occur on the stack, which may be exploitable. This occurs in spite of the outer function's attempt to prevent buffer overflow! Wiki MarkupThis code fixes one of the two problems from the previous non-compliant code example: it eliminates the possibility of buffer overflow because two references to {{msg}} in {{strcpy_s()}} both refer to {{msg\[80\]}} defined in the subscope. The initial problem of not changing the value of the outside {{msg}} variable value remains. The call to {{strcpy_s()}} will also fail if the length of the null-terminated byte string referenced by {{error_msg}} is longer than 79 characters in length.

Compliant Solution

This compliant solution uses different, more descriptive variable names. Also it uses strcpy_s().

Code Block
bgColor#ccccff
char system_msg[100];
/* ... */
void error_message(char *error_msg) {
  char default_msg[80];
  /* ... */

  /* Assume error_msg isisn't assumedtoo tolong reference*/
 a NTBS of length 99 or less strcpy(system_msg, error_msg);
  return;
}
/* ... */

/* Ensure errnoerror_msg isn't e = strcpy_s(system_msg,too long */
if (strlen( error_msg) >= sizeof( system_msg),) {
  error_msg[sizeof(msg);
  if (e !- 1] = 0) {
     '\0';
}
error_message( error_msg); /* handle strcpy_s() error good */
  }
}

When the block is small, the danger of reusing variable names is mitigated by the visibility of the immediate declaration. Even in this case, however, variable name reuse is not desirable.

By using different variable names globally and locally, the compiler forces the developer to be more precise and descriptive with variable names.

Risk Assessment

Reusing a variable name in a subscope can lead to unintended values for the variable.

...