FIO37-C. Do not assume that fgets() returns a nonempty string when successful

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 recommendation 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.

C99, Section 7.19.7.2, "The fgets() function", paragraph 3 says

The fgets function returns s if successful. If end-of-file is encountered and no characters have been read into the array, the contents of the array remain unchanged and a null pointer is returned.

Therefore, if fgets() returns a non-null pointer, we can assume it actually filled the array with data. However, assuming that it filled the array with a non-empty null-terminated byte string (NTBS) is erroneous because the data it placed in the array may contain null characters.

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() happens to be a null character. This may occur, for example, if a binary data file is read by the fgets() call [Lai 2006]. 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 rue 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 an out-of-bounds memory write.

Rule	Severity	Likelihood	Remediation Cost	Priority	Level
FIO37-C	high	probable	medium	P12	L1

Automated Detection

Tool	Version	Checker	Description
Fortify SCA	V. 5.0
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.

Related Guidelines

CERT C++ Secure Coding Standard: FIO37-CPP. Do not assume character data has been read

ISO/IEC 9899:1999 Section 7.19.7.2, "The fgets function"

MITRE CWE: CWE-119, "Failure to Constrain Operations within the Bounds of an Allocated Memory Buffer"

MITRE CWE: CWE-241, "Failure to Handle Wrong Data Type"

Bibliography

[Lai 2006]
[Seacord 2005a] 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

Space shortcuts

Page tree