FIO37-C. Do not assume character data has been read

Errors can occur when assumptions are made about the type of data being read. These assumptions may be violated, for example, when binary data has been read from a file instead of text from a user's terminal (see FIO14-C. Understand the difference between text mode and binary mode with file streams). On some systems, it may also be possible to input a null byte (as well as other binary codes) from the keyboard.

Noncompliant Code Example

This noncompliant code example attempts to remove the trailing new-line (\n) from an input line. The fgets() function is typically used to read a new-line terminated-line of input from a stream. It takes a size parameter for the destination buffer and copies, at most, size-1 characters from a stream to a character array.

char buf[BUFSIZ + 1];

if (fgets(buf, sizeof(buf), stdin) == NULL) {
  /* Handle error */
}
buf[strlen(buf) - 1] = '\0';

The strlen() function computes the length of a string by determining the number of characters that precede the terminating null character. A problem occurs if the first character read from the input by fgets() is a null character. This may occur, for example, if a binary data file is read by the fgets() call [[Lai 06]]. If the first character in buf is a null character, strlen(buf) returns 0 and a write-outside-array-bounds error occurs.

Compliant Solution

This compliant solution uses strchr() to replace the new-line character in the string, if it exists (see FIO36-C. Do not assume a new-line character is read when using fgets()).

char buf[BUFSIZ + 1];
char *p;

if (fgets(buf, sizeof(buf), stdin)) {
  p = strchr(buf, '\n');
  if (p) {
    *p = '\0';
  }
}
else {
  /* Handle error condition */
}

Risk Assessment

Assuming character data has been read can result in out-of-bounds memory writes.

Automated Detection

Fortify SCA Version 5.0 can detect violations of this rule.

Compass/ROSE could detect some violations of this rule. In particular, it could detect the NCCE by searching for fgets(), followed by "strlen() - 1", which could be -1. The crux of this rule is that a string returned by fgets() could still be empty, because the first char is '\0'. There are probably other code examples that violate this guideline; we would need to enumerate them before ROSE could detect them.

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Other Languages

This rule appears in the C++ Secure Coding Standard as FIO37-CPP. Do not assume character data has been read.

References

[[ISO/IEC 9899:1999]] Section 7.19.7.2, "The fgets function"
[[Lai 06]]
[[MITRE 07]] CWE ID 119, "Failure to Constrain Operations within the Bounds of an Allocated Memory Buffer," CWE ID 241, and "Failure to Handle Wrong Data Type"
[[Seacord 05a]] Chapter 2, "Strings"

FIO36-C. Do not assume a new-line character is read when using fgets()      09. Input Output (FIO)       FIO38-C. Do not use a copy of a FILE object for input and output