Hello.

Le lun. 7 juin 2021 à 06:51, Stefan Bodewig <bode...@apache.org> a écrit :
>
> On 2021-06-06, Gilles Sadowski wrote:
>
> > Le dim. 6 juin 2021 à 07:51, Stefan Bodewig <bode...@apache.org> a écrit :
>
> >> Hi
>
> >> I'm thinking about a specific IOException subclass that is thrown when a
> >> RuntimeException "happens" somewhere in the code that parses data in
> >> Zip/SevenZ/TarFile, see
>
> > I'm afraid I missed part of the story as to what is the original problem.
>
> Sorry, I should have expanded on that.
>
> When we uncompress a stream / expand an archive our users most of the
> time are not responsible for the input. If the data they hand over to
> Compress is invalid, they expect the library to throw an IOException -
> and in many cases this is true.

Does "Compress" make the promise that all exceptions are
(subclasses) of IOException?

> But the reality is most of our parsing code is written for the good case
> where the archive follows the spec. The code relies on numbers to be
> where they should be and not letters, it may fail to check an offset is
> inside of the bounds of an array and so on. So for certain types of
> broken archives the parsers will throw arbitrary RuntimeException
> (NumberFormat, ArrayIndexOutOfBounds, NegativeArraySize and so on).
>
> People do not expect said RuntimeExcepitons, so they don't catch them.

If the answer to the above question was "no", then it's their
responsibility to protect their code against RuntimeException.

> In a situation where an attacker controls the input this can be used to
> make the application reading it crash. So for certain types of
> applications this might be security relevant, it could be a DoS
> vector. Of course one can argue the calling code should better protect
> itself when it reads untrusted input and catch even undeclared
> exceptions,

I cannot see of how one could argue differently.
That runtime exceptions can occur is a Java fact.

> that's why we've never issued CVEs for exceptions where our
> parser code has not been strict enough in the past.
>
> After Compress 1.20 we've had lots of reports of RuntimeExceptions being
> thrown, many of which have been uncovered by fuzz testing tools (not
> only, but also OSS Fuzz).
>
> What I suggest is a stop-gap. It is not an excuse for not properly
> verifying input in our parsing code

I still don't think that "Compress" should double-check a condition
(e.g. array index, null, ...) that will be checked at some lower level
and whose worst consequence would be to throw an exception.

> but rather a way to limit the impact
> of such oversights.

If the answer to the above question is "yes", then AFAICT the
only fix is what you proposed.

Best regards,
Gilles

>
> Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to