RFR 8183591: Incorrect behavior when reading DER value with Integer.MAX_VALUE length

[Adam Petcher]

On 7/20/2017 1:32 PM, Bernd wrote:

> Why not make a different utility method for this case.
>
>  readRemaining() vs. readFully(int)
>
> The name makes not much sense and the code does not get easier if both 
> cases are in one method for no good reason.

I agree, but the method that takes a length argument is the only one 
that is needed at the moment. We can add another method later if it is 
needed. Are you saying there is something wrong with the readFully(int) 
method in my last webrev?

>
> And I wonder if allocating a MAXINTEGER buffer from untrusted source 
> is a good idea.

This method will only allocate a large buffer if the untrusted source 
actually sent a large amount of bytes.

>
> Gruss
> Bernd
>
> 2017-07-20 15:49 GMT+02:00 Adam Petcher <adam.petcher at oracle.com 
> <mailto:adam.petcher at oracle.com>>:
>
>     Oops. Better to throw an IOException when a negative length is
>     given to readFully.
>
>     Webrev: http://cr.openjdk.java.net/~apetcher/8183591/webrev.02/
>     <http://cr.openjdk.java.net/%7Eapetcher/8183591/webrev.02/>
>
>
>
>     On 7/18/2017 1:55 PM, Adam Petcher wrote:
>
>         Some additional investigation revealed that
>         IOUtils.readFully() is only used by DER, JKS, and Kerberos.
>         None of these need the "read to the end of the buffer"
>         feature. This behavior of readFully() is confusing, so it is
>         probably best to remove it.
>
>         Webrev:
>         http://cr.openjdk.java.net/~apetcher/8183591/webrev.01/
>         <http://cr.openjdk.java.net/%7Eapetcher/8183591/webrev.01/>
>
>
>         On 7/12/2017 2:38 PM, Adam Petcher wrote:
>
>             This is a bug fix for a corner case in which a DER value
>             has length equal to Integer.MAX_VALUE. The code uses
>             IOUtils.readFully() to read the value, which interprets
>             length=Integer.MAX_VALUE to mean "read to the end." The
>             result is that no exception will be thrown when fewer then
>             Integer.MAX_VALUE bytes are read from the stream. The fix
>             adds a check after the readFully() to ensure that the
>             expected number of bytes were read.
>
>             Webrev:
>             http://cr.openjdk.java.net/~apetcher/8183591/webrev.00/
>             <http://cr.openjdk.java.net/%7Eapetcher/8183591/webrev.00/>
>             JBS: https://bugs.openjdk.java.net/browse/JDK-8183591
>             <https://bugs.openjdk.java.net/browse/JDK-8183591>
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170720/1bd3165f/attachment.htm>