Potential IndexOutBounds in AbstractServletInputStream::readLine() ?

[Oswaldo Olivo]

*** Re-sending because I think I used the wrong e-mail to send the message
to the list ***
Hi,

I'm using Tomcat 7.0.60, building it up from source.

I was wondering if there is an unintentional potential index of out bounds
exception in AbstractServletInputStream::readLine() ?
I was looking at the code in java/org/apache/coyote/http11/upgrade/
AbstractServletInputStream , specifically the readLine() function:

=========================
 public final int readLine(byte[] b, int off, int len) throws IOException {
        preReadChecks();

if (len <= 0) {
            return 0;
        }
    int count = 0, c;

        while ((c = readInternal()) != -1) {
            b[off++] = (byte) c;
    count++;
            if (c == '\n' || count == len) {
                break;
            }
        }
        return count > 0 ? count : -1;
    }
=========================

It seems that "len" is partially sanitized, but the offset parameter 'off'
is not.

In particular, 'off' could be allowed to be outside of 'buf', causing an
exception while executing the statement b[off++]=(byte)c;

The way the inner conditionals are implemented, it seems that also starting
with a valid offset, but a large value of len can also cause this exception.

One could change the loop condition to something like
"((c=readInternal())!= -1 && 0<=off && off<b.length)"

This function seems to be inherited by concrete classes
AprServletInputStream and BioServletInputStream without being overriden.

I believe that the implementation of readLine() in javax.ServletInputStream
handles these border cases by returning -1 whenver an access outside of the
array is attempted, so it doesn't suffer from this problem.

Is this an issue that needs to be changed or is it the intended behavior to
leave the responsibility of sanitizing the parameters to the caller ?

Thanks,
 Oswaldo.