On Sat, 9 Dec 2023 12:14:35 GMT, Bernd <d...@openjdk.org> wrote:

>> @stsypanov Yes but still it is just weird to ask any output stream if *it* 
>> is trusted. I mean, it feels just unsecure to ask: "Do *you* pretend to be 
>> trusted?" instead of "Do *we* trust you?". I could sleep better if this 
>> method would not be part of each OutputStream subclass. We should either 
>> move it back to the place where needed, or into some static utility like 
>> `OutputStreams::isTrusted(OutputStream)` (mind the plural!), or it should at 
>> least be `final`.
>
> (Deleted) the new version doesn’t have the issue (albeit now it’s rather 
> complicated formulated) “If stream can be used by JCL callers without extra 
> copies of the byte[] argument to write(). This is not over writeable by user 
> code.”

Unlike BAIS, the BufferedInputStream can wrap an untrusted InputStream.  
BIS.buf is passed directly to wrapped InputStream so I would assume that we 
would want to avoid exposing BIS.buf to the `out` parameter of transferTo.  
This way we know the input stream is not able to poison the output stream when 
a write is in progress.  

I assume that the current small list of trusted classes are also immune to 
poison so I imagine this patch is safe.  However, for any FilterInputStream we 
should either always use Arrays.copyOfRange because the input side can poison 
the output side or it needs a mirroring allow list for the target input stream 
to insure that the wrapped input stream is not poisoning the out parameter.

For instance, java.util.zip.CheckedOutputStream in theory could be added as 
trusted class as it doesn't leak or poison but, looking at the code it would 
appear that it is not immune to poison.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1421607704

Reply via email to