RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v5]

jmehrens duke at openjdk.org
Sat Dec 9 23:40:13 UTC 2023


On Sat, 9 Dec 2023 12:14:35 GMT, Bernd <duke at 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


More information about the core-libs-dev mailing list