Re: Potential IndexOutBounds in AbstractServletInputStream::readLine() ?
Hi,
Thanks for you detailed answer.
I guess I was misunderstanding the specification.
Regards,
Oswaldo.
On Sat, Mar 7, 2015 at 1:47 PM, Christopher Schultz
ch...@christopherschultz.net wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Oswaldo,
On 3/4/15 12:10 AM, Oswaldo Olivo wrote:
Hi, 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 offb.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 ?
I think this has already been answered elsewhere, but just in case..
This implementation is as intended. If the caller doesn't send sane
parameters to this method, it will throw an exception. There is no
reason to waste time double-checking the parameters because the
failure mode is mild: an exception is thrown. This isn't like C where
failure to check array boundaries can result in security vulnerabilities.
The runtime performs bounds-checking, so the code would just be
wasting time checking the same.
Finally, your suggestion for how to handle out-of-bounds violations
would violate the API contract for how the method should behave with
respect to return values.
- -chris
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - http://gpgtools.org
iQIcBAEBCAAGBQJU+1XPAAoJEBzwKT+lPKRYi38P/05pyE/7gxYqbEYMB9iHGwru
w7KCj5NzFy+DlDHjeTedY07THTfCI3WnIgnLWZia0tkwDO06O5QDFOrkonln6wwv
7Jh4Zu/iXMCKRGGZmYggkXQ5kfbjkQstcOFri3XqvUCWC/mmiXaABkatpNh/vr/P
oslmuxj76quj2vH0sFT/i9Gf5AaLPc4wuO1uGSfzdr0dv/wMoN9GGmT/MEbK+NjW
Zq7pOX0Lvs2pBIiYos0K21NmPhsm1TWyl2WXNgJAODHUpdXtO6m9jQ5UPPVbUv/A
FbNpEqsJpjSvVBL9dU1ChYCZuoqkCbyW4nI/IphtM2ZAsmAoIXeuNxITQ1bRaTbB
13v5y9lZ7uxkPP/gl7ovAGyJ5qAN3O4Q1jnGcGnzF1RDU2lL9NPfoAWkmk1mcDwp
g1nbc4OgG1q5mykLFPKG7qpyW+T2nFXtWuCs0zDryxdVBBLwhNhPDxcSc1us3w3t
AQlFa32zbHFI3KYUcaGQoJv/OLsqO3ARCfIZDJqTcxro/sYnJ9Nb0UeAASKRmRLY
Yzx6Kajr1rfc3IYHC5+lCxpUr2sJSHRxWzZAbkcmSXB0KBrSsGZrRsdKTPCLlS/Y
nVFQ/zzHMx0CvmpfYZp67C3ZX+V/0dNn0P5O8dDQmnERfaJzijinC39fTNBnMwW3
Jf+xA9hEgk/+CtoupArw
=yZme
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Potential IndexOutBounds in AbstractServletInputStream::readLine() ?
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Oswaldo,
On 3/4/15 12:10 AM, Oswaldo Olivo wrote:
Hi, 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 offb.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 ?
I think this has already been answered elsewhere, but just in case..
This implementation is as intended. If the caller doesn't send sane
parameters to this method, it will throw an exception. There is no
reason to waste time double-checking the parameters because the
failure mode is mild: an exception is thrown. This isn't like C where
failure to check array boundaries can result in security vulnerabilities.
The runtime performs bounds-checking, so the code would just be
wasting time checking the same.
Finally, your suggestion for how to handle out-of-bounds violations
would violate the API contract for how the method should behave with
respect to return values.
- -chris
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: GPGTools - http://gpgtools.org
iQIcBAEBCAAGBQJU+1XPAAoJEBzwKT+lPKRYi38P/05pyE/7gxYqbEYMB9iHGwru
w7KCj5NzFy+DlDHjeTedY07THTfCI3WnIgnLWZia0tkwDO06O5QDFOrkonln6wwv
7Jh4Zu/iXMCKRGGZmYggkXQ5kfbjkQstcOFri3XqvUCWC/mmiXaABkatpNh/vr/P
oslmuxj76quj2vH0sFT/i9Gf5AaLPc4wuO1uGSfzdr0dv/wMoN9GGmT/MEbK+NjW
Zq7pOX0Lvs2pBIiYos0K21NmPhsm1TWyl2WXNgJAODHUpdXtO6m9jQ5UPPVbUv/A
FbNpEqsJpjSvVBL9dU1ChYCZuoqkCbyW4nI/IphtM2ZAsmAoIXeuNxITQ1bRaTbB
13v5y9lZ7uxkPP/gl7ovAGyJ5qAN3O4Q1jnGcGnzF1RDU2lL9NPfoAWkmk1mcDwp
g1nbc4OgG1q5mykLFPKG7qpyW+T2nFXtWuCs0zDryxdVBBLwhNhPDxcSc1us3w3t
AQlFa32zbHFI3KYUcaGQoJv/OLsqO3ARCfIZDJqTcxro/sYnJ9Nb0UeAASKRmRLY
Yzx6Kajr1rfc3IYHC5+lCxpUr2sJSHRxWzZAbkcmSXB0KBrSsGZrRsdKTPCLlS/Y
nVFQ/zzHMx0CvmpfYZp67C3ZX+V/0dNn0P5O8dDQmnERfaJzijinC39fTNBnMwW3
Jf+xA9hEgk/+CtoupArw
=yZme
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Potential IndexOutBounds in AbstractServletInputStream::readLine() ?
I see. Thank you! -- Oswaldo. On Wed, Mar 4, 2015 at 4:21 PM, Caldarale, Charles R chuck.caldar...@unisys.com wrote: From: Oswaldo Olivo [mailto:ozzy...@gmail.com] Subject: Potential IndexOutBounds in AbstractServletInputStream::readLine() ? I was wondering if there is an unintentional potential index of out bounds exception in AbstractServletInputStream::readLine() ? It's not unintentional. It seems that len is partially sanitized, but the offset parameter 'off' is not. As the spec requires. In particular, 'off' could be allowed to be outside of 'buf', causing an exception while executing the statement b[off++]=(byte)c; Which is an error by the caller, resulting in an exception. One could change the loop condition to something like ((c=readInternal())!= -1 0=off offb.length) For what purpose? The return value of -1 specifically means there is no more data to be read. 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. Presumably you meant javax.servlet.ServletInputStream, not what you wrote. The readLine() implementation for that class certainly does not do what you describe, nor should it. Read the servlet spec and JavaDoc. 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 ? Nothing in the spec indicates that the current behavior is inappropriate. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers. - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Potential IndexOutBounds in AbstractServletInputStream::readLine() ?
*** 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 offb.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.
RE: Potential IndexOutBounds in AbstractServletInputStream::readLine() ?
From: Oswaldo Olivo [mailto:ozzy...@gmail.com] Subject: Potential IndexOutBounds in AbstractServletInputStream::readLine() ? I was wondering if there is an unintentional potential index of out bounds exception in AbstractServletInputStream::readLine() ? It's not unintentional. It seems that len is partially sanitized, but the offset parameter 'off' is not. As the spec requires. In particular, 'off' could be allowed to be outside of 'buf', causing an exception while executing the statement b[off++]=(byte)c; Which is an error by the caller, resulting in an exception. One could change the loop condition to something like ((c=readInternal())!= -1 0=off offb.length) For what purpose? The return value of -1 specifically means there is no more data to be read. 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. Presumably you meant javax.servlet.ServletInputStream, not what you wrote. The readLine() implementation for that class certainly does not do what you describe, nor should it. Read the servlet spec and JavaDoc. 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 ? Nothing in the spec indicates that the current behavior is inappropriate. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers. - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Potential IndexOutBounds in AbstractServletInputStream::readLine() ?
Hi,
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 offb.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.
