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

[jmehrens]

On Thu, 30 Nov 2023 23:27:13 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Good catch: that in fact defeats the protection.
>
> 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);


Which leads us back to an allow list that Alan was talking about and the draw backs that it brings.

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

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