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 < [email protected]> 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 && 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 ? > > 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: [email protected] > For additional commands, e-mail: [email protected] > >
