Mailing-List: contact users-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Users List" <users@tomcat.apache.org>
Received-SPF: pass (athena.apache.org: domain of ozzyo86@gmail.com designates
 74.125.82.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <54FB55CF.6070302@christopherschultz.net>
References: 
 <CAF7Yjpqs_9xd=K0Ja6cj1Z8Fr3GsQGtjoKzAAbtAvtE8pbQ9xw@mail.gmail.com>
	<54FB55CF.6070302@christopherschultz.net>
Date: Mon, 9 Mar 2015 03:03:46 -0500
Message-ID: 
 <CAF7YjproTUvdWEtNnh2qEsbPH=chBzv1iYGgno3hk9g6-sD6PQ@mail.gmail.com>
Subject: Re: Potential IndexOutBounds in
 AbstractServletInputStream::readLine()
 ?
From: Oswaldo Olivo <ozzyo86@gmail.com>
To: Tomcat Users List <users@tomcat.apache.org>
Content-Type: multipart/alternative; boundary=089e01184dd638c9c20510d67958

--089e01184dd638c9c20510d67958
Content-Type: text/plain; charset=UTF-8

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 <
chris@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 && 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: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>

--089e01184dd638c9c20510d67958--