RFR: 8321053: Use ByteArrayInputStream.buf directly when parameter of transferTo() is trusted [v2]

[Brian Burkhalter]

On Fri, 1 Dec 2023 01:19:32 GMT, jmehrens <duke at openjdk.org> wrote:

>> Changed in 176d5165f7d8f3fa4814c9838abb5d18d9f3c338 not to trust `FilterOutputStream`s.
>
> The only other alternative would be to walk `((FilterOutputStream)out).out` and if everything in the out chain is in the "java." package then the out can be trusted.
> 
> byte[] tmp = null;
> for (OutputStream os = out; os != null;) {
>     if (os.getClass().getPackageName().startsWith("java.")) {
>         if (os instanceof FilterOutputStream fos) {
>             //loops in this chain is going to cause this code to never end.
>             // self reference A -> A or transitive reference A -> B -> C ->A
>             os = fos.out;
>             continue;
>         }
>         break;
>     }
>             
>     tmp = new byte[Integer.min(len, MAX_TRANSFER_SIZE)];
>     break;
> }
> 
> I don't like the approach of deny list, walking the chain as (subjectively) it seems too fragile.
> 
> Also I think I can break this version of the code with ChannelOutputStream. I didn't run this through a compiler nor test it but the idea is that ChannelOutputStream calls ByteBuffer.wrap(bs) and doesn't call ByteBuffer.asReadOnlyBuffer.  So a malicious WritableByteChannel should be able to gain access to the original array:
> 
> WritableByteChannel wolf = new WritableByteChannel() {
> public int write(ByteBuffer src) throws IOException {
>       src.array()[0] = '0'; //oh no!
>       return 0;
>  }
> };
> 
> ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
> OutputStream wolfInSheepSuitAndTie = Channels.newOutputStream(wolf);
> bais.transferTo(wolfInSheepSuitAndTie);
> 
> However, the ChannelOutputStream is in sun.nio.ch so on second thought it shouldn't break.  The pattern is repeated in Channels.newOutputStream(AsynchronousByteChannel ch) so that should fail as it is in the "java." namespace.
> 
> I think an allow list would be safer but that brings all the drawbacks that Alan was talking about before.

I might have done this incorrectly, but with this version of the above `wolf` I do not see any corruption:

        java.nio.channels.WritableByteChannel wolf =
            new java.nio.channels.WritableByteChannel() {
                private boolean closed = false;
                
                public int write(java.nio.ByteBuffer src) throws IOException {
                    int rem = src.remaining();
                    Arrays.fill(src.array(), src.arrayOffset() + src.position(),
                                src.arrayOffset() + src.limit(),
                                (byte)'0');
                    src.position(src.limit());
                    return rem;
                }

                public boolean isOpen() {
                    return !closed;
                }

                public void close() throws IOException {
                    closed = true;
                }
            };

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16893#discussion_r1411539285