FIO14-J. Use an int to capture the return value of functions that read a character or byte

The abstract InputStream.read() method is defined as follows, according to [[API 2006]]:

Reads the next byte of data from the input stream. The value byte is returned as an int in the range 0 to 255. If no byte is available because the end of the stream has been reached, the value -1 is returned. This method blocks until input data is available, the end of the stream is detected, or an exception is thrown.

This method is often used to read a byte from an input stream. It is sometimes also used to read a character from the input stream when the character is known to require only 8 bits. Unfortunately many programmers eagerly convert the resulting int to a byte or char before checking the value for -1. It is vital to check that read() did not return -1 before narrowing the return value to a byte or char.

This guideline applies to any InputStream subclass that provide an implementation of the read() method. This guideline is a specific instance of NUM15-J. Ensure conversions of numeric types to narrower types do not result in lost or misinterpreted data.

Noncompliant Code Example (byte)

This noncompliant code example casts the value returned by the read() method directly to a value of type byte, and then compares this value with -1 in an attempt to detect the end of the stream.

FileInputStream in;
// initialize stream 
byte data;
while ((data = (byte) in.read()) != -1) { 
  // ... 
}

If the read() method in this noncompliant code example returns the byte value 0xFF, it will be indistinguishable from the -1 value used to indicate the end of stream, because the byte value is promoted and sign extended to an int before being compared with -1.

Compliant Solution (byte)

Use and value of type int to capture the return value of the byte input method. If the value returned by read() is not -1, it can be safely cast to type byte. If read() returns 0XFF, the comparison will test 0x000000FF against 0xFFFFFFFF and fail.

FileInputStream in;
int inbuff;
byte data;
while ((inbuff = in.read()) != -1) { 
  data = (byte) inbuff;
  // ...  
}

Noncompliant Code Example (char)

This noncompliant code example casts the value of type int returned by the read() method directly to a value of type char, which is then compared with -1 in an attempt to detect the end of stream. This conversion leaves the value of c as 0xffff (Character.MAX_VALUE) instead of -1. As a result, the test for the end of stream never evaluates to true.

char c;
while ((c = (char) in.read()) != -1) { 
  // ... 
}

Compliant Solution (char)

Use and value of type int to capture the return value of the character input method. When the value of returned by read() is not -1, it can be safely cast to type char.

int inbuff;
char data;
while ((inbuff = in.read()) != -1) { 
  data = (char) inbuff;
  // ...  
}

Risk Assessment

Historically, using a narrow type to capture the return value of a byte input function has resulted in significant vulnerabilities, including command injection attacks. (See the CA-1996-22 advisory.) As a result, the severity of this error is high.

Automated Detection

FindBugs version 1.3.9 can detect violations of this guideline with the INT: Bad comparison of nonnegative value with negative constant detector.

Related Vulnerabilities

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

Related Guidelines

C Secure Coding Standard: FIO34-C. Use int to capture the return value of character IO functions

C++ Secure Coding Standard: FIO34-CPP. Use int to capture the return value of character IO functions

Bibliography

[[API 2006]] Class InputStream
[[JLS 2005]] Section 4.2 "Primitive Types and Values"
[[Pugh 2008]] "Waiting for the end"

NUM17-J. Beware of precision loss when converting primitive integers to floating-point      03. Integers (INT)      NUM04-J. Use shift operators correctly