Generic exception catching in Java fuzz tests about oss-fuzz

[oliverchang]

I think that's likely reasonable most of the time, but it does make a bit of a wide assumption which may hide legitimate DoS issues that the project may care about.

@DavidKorczynski @AdamKorcz any thoughts?

[DavidKorczynski]

Is it okay to catch and ignore any other exception apart from the ones mentioned above?

I think this is quite context dependent and I would not go so wide and say any other exception can be ignored.

Although your question is broader than RuntimeExceptions, let me try and answer by way of using RuntimeExceptions as an eample -- RuntimeExceptions are a per-project topic perhaps. However, below are examples of RuntimeExceptions in popular libraries found by OSS-Fuzz and then fixed, so there is incentive across some projects to fix these issues.

• apache/commons-codec#219
• apache/commons-codec#219
• apache/commons-codec#220
• apache/commons-codec#221
• apache/commons-codec#223
• apache/commons-lang#1140
• apache/commons-lang#1145
• FasterXML/jackson-dataformat-xml#619
• FasterXML/jackson-datatypes-collections#125
• FasterXML/jackson-dataformats-binary#461
• FasterXML/jackson-dataformats-binary#459

However, sometimes fixes are not desirable as the code is meant to throw RuntimeExceptions, and then maybe documentation is needed apache/commons-lang#1139

I'm not an expert in the area, but in general I think it comes down to whether the RuntimeExceptions are meant to be there or not, and if they are meant to be there then I think it's important to document the presence of the exceptions (at least if it's a library) so anyone can code according to the interface. Some of the fixes above are also simply throwing a specific RuntimException rather than fixing that an exception is thrown -- i.e. going from an "uncontrolled" RuntimeException to a "controlled" RuntimeException.

If there are undocumented RuntimeExceptions in the target code, then that can lead to weird program states, e.g. it may be that it's possible to enabling something in the target program that will always trigger a runtime exception and this will then cause any application using the given library to not work properly, resulting in some denial of service.

Once we have specified all of the "legit" RuntimeExceptions a piece of code can throw, we then adjust the fuzzing harnesses accordingly:

• oss-fuzz/projects/jackson-datatype-joda/JodaSerializerFuzzer.java

  Line 88 in 29b1bbe

  } catch (IllegalArgumentException | ArithmeticException | JsonProcessingException e) {

• oss-fuzz/projects/jackson-datatype-joda/JodaMapperFuzzer.java

  Line 61 in 29b1bbe

  } catch (JsonProcessingException | IllegalArgumentException | ArithmeticException e) {}

• oss-fuzz/projects/jackson-dataformat-xml/XmlSerializerFuzzer.java

  Line 141 in 29b1bbe

  } catch (IOException | NumberFormatException e) {

• https://github.com/google/oss-fuzz/blob/29b1bbe08d45b44d48a4f0f220be6001000b9b93/projects/apache-commons-codec/NetCodecFuzzer.java#L89C52-L89C77
  ....

[DavidKorczynski]

@arthurscchan any thoughts on this?

[arthurscchan]

General RuntimeException and FuzzerSecurityIssueXXX from Jazzer are quite different IMO, those RuntimeException could be caught easily. But those FuzzerSecurityIssueXXX are not that easy to be caught in the fuzzer logic since most of the FuzzerSecurityIssueXXX are thrown from injected code by Jazzer with the reflection API. Thus the main logic in the fuzzer is not aware of them during compile time. For example, Jazzer injects a hook method call before any regular expression execution and checks if the string passing to the real execution is vulnerable to REDOS, as shown in https://github.com/CodeIntelligenceTesting/jazzer/blob/5bc43a6f65180cb003605349c9e2fdadc702d2c8/sanitizers/src/main/java/com/code_intelligence/jazzer/sanitizers/RegexInjection.kt#L114-L163. In this kind of situation, even catching the exception in the fuzzer does not help because that FuzzerSecurityIssueLow is thrown by the jazzer engine, not the fuzzer itself.

To summarize, those FuzzerSecurityIssueXXX exceptions could be caught but may need further changes in the infra to activate or deactivate the handling of these exceptions. These activations or deactivations could be configurable via the project.yaml file. It could also be configured in more detail on which specific FuzzerSecurityIssueXXX in which fuzzers are meant to be ignored. But I could foresee it is not a small change. Also, it may result in a false negative when some of these exceptions are ignored.

As a whole, I think it is a balance between the usability of fuzzers and the ratio of false-positive/false-negative bug discovery. But I think it may be a good idea to add this flexibility and allow the owner of each oss-fuzz project to decide whether any of those FuzzerSecurityIssueXXX exceptions could be added.

[arthurscchan]

Oh..... I may have complicated and over-interpreted the question......... The simple answer is, NO.
Java has two categories of throwable objects, one is Exceptions and the other is Errors. And in exceptions are separated into RuntimeException and others, RuntimeException does not need to be handled during compile time, but the catch is, that there is some "security sensitive" RuntimeException, like java.lang.SecurityException. Thus I don't think a simple catch of all exceptions is a good way, it will create false negatives easily. In addition, even the simple IndexOutOfBoundsException could cause security problems if the array or string are used for security purposes, like credentials validation.

[jam1729]

Hi @DavidKorczynski @arthurscchan @oliverchang,
Thank you very much :) for your thoughts!
Thanks David and Arthur for your examples.

I understand that catching and ignoring any Exception is a bad idea, because RuntimeExceptions could be possible vulnerabilities based on the context of the package. I also understand that even if it is not a vulnerability, there is still value addition in documenting them.

With the above understanding - can we conclude that it is considerably safe to catch and ignore all the known exceptions?

By known, we include -

1. Any checked exception (defined in the function's throws clause)
2. Unchecked exceptions defined in the function's docsting, if present (example)
3. Any exception explicitly thrown by the function, even if it is not mentioned in the docstring.
4. (did I miss other known exceptions?)

Points 2 and 3 seem a little tricky because exceptions that are added in the docstring or explicitly thrown may be intended to be thrown only on a particular case (example) and not on all cases. If we catch and ignore this exception without checking why this exception occurred, we might possibly be ignoring a possible vulnerability. Is that right?

Thanks!

[irf-ali]

Oh..... I may have complicated and over-interpreted the question......... The simple answer is, NO. Java has two categories of throwable objects, one is Exceptions and the other is Errors. And in exceptions are separated into RuntimeException and others, RuntimeException does not need to be handled during compile time, but the catch is, that there is some "security sensitive" RuntimeException, like java.lang.SecurityException. Thus I don't think a simple catch of all exceptions is a good way, it will create false negatives easily. In addition, even the simple IndexOutOfBoundsException could cause security problems if the array or string are used for security purposes, like credentials validation.

@oliverchang ClusterFuzz reports have a flag which says if it is a security issue or not. In all the reports I have seen, the security issues were always crashing with the FuzzerSecurityIssue exception. Other uncaught exceptions don't seem to be marked as security issues. This is why we assumed that only FuzzerSecurityIssue exceptions are security issues.
Do you have any example of a vulnerability caught by OSS Fuzz which did not crash with FuzzerSecurityIssue exception?

[DavidKorczynski]

an we conclude that it is considerably safe to catch and ignore all the known exceptions?

By known, we include -

1. Any checked exception (defined in the function's throws clause)
2. Unchecked exceptions defined in the function's docsting, if present (example)
3. Any exception explicitly thrown by the function, even if it is not mentioned in the docstring.
4. (did I miss other known exceptions?)

Yes, we can conclude this and I think it's a good summary of it.

Points 2 and 3 seem a little tricky because exceptions that are added in the docstring or explicitly thrown may be intended to be thrown only on a particular case (example) and not on all cases. If we catch and ignore this exception without checking why this exception occurred, we might possibly be ignoring a possible vulnerability. Is that right?

That's correct and a good point you highlight. When we adjust our fuzzers to catch exceptions it may be be an over-approximation if only some parts of the target code is meant to be able to throw the given exception.