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