On 2021-05-24, Bernd wrote: > Am Mo., 24. Mai 2021 um 20:46 Uhr schrieb Matt Sicker <[email protected]>:
>> There's also a bit of an issue of fixing these types of >> vulnerabilities at the library level. The library itself typically >> won't have much in the way of a security model until you integrate it >> into an application. > That is true in general, but we have for the found issues (which are not > OOM) two possibilities, catch the runtime exceptions and rethrow them as a > IOException (Maybe a new subtype like MalformedArchiveException extends > IOException) or document in javadoc that the runtime exceptions dealing > with Out of Bounds and maybe NullPointers might be thrown. Do you think > this is a good idea? So far we've preferred the "throw an IOException" approach in Compress. In some areas like handling of ZIP extra fields we're simply catching all RuntimeExcepiton and rethrow them as IOExceptions, in most other cases we try to identify the cases where a RuntimeException would occur and throw an IOException prior to that. > - I can prepare a patch for it, would prefer a new specific sub > exception. My changes so far have focussed on avoiding the exception alltogether rather than "catch everything at the API entry point" as I was hoping to catch errors early and avoid OOMs and infinite loops by more aggressive bounds checks. Some of Compress' packages have got a big public API for historic reasons. The tar package is probably the worst offender with TarUtils having lots of methods that are public for no good reason, some of which don't even declare any checked exceptions to be thrown. Here "document the exception" is the only thing you can do - and catch/rethrow inside of the other code parts that call them. > Having said that, it is not uncommon that a size field is used to allocate > Buffers, in that case an OOM is possible and a Limit Manager would be > helpful. This does not only help against malicious files. Not sure if the > fuzzer wil find that in the future... Some of our packages support a custom option that says "don't use more than X MB", but this is limited to a few algorithms that do provide estimates like LZMA and XZ. So far OSS Fuzz is not setting this option for anything (I believe this only applies to the 7z case currently) so OSS Fuzz could run into OOMs that users could avoid by setting the given option - something we could change if we wanted to. Stefan --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
