On Fri, 1 Dec 2023 01:19:32 GMT, jmehrens <d...@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
