diff options
author | Fabian Meumertzheim <meumertzheim@code-intelligence.com> | 2021-03-10 22:28:02 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-10 13:28:02 -0800 |
commit | 3a227bd77da9ca96155e76b605bf4cae9cf7f8e7 (patch) | |
tree | d505500496ca4341a76edc5a03cfd5ede8dab281 /projects | |
parent | 53e9531551ff6438693fd3690403e809aa7ce221 (diff) | |
download | oss-fuzz-3a227bd77da9ca96155e76b605bf4cae9cf7f8e7.tar.gz |
[json-sanitizer] Add severity markup (#5350)
Annotates the findings of the various json-sanitizer fuzzers with
severities as follows:
* XSS: High
* Comment injection: Medium
* Invalid JSON: Low
* Failure to be idempotent: Not a security issue
* Undeclared exceptions: Not a security issue
This commit takes advantage of the support for severity markers in stack
traces introduced in https://github.com/google/clusterfuzz/pull/2270.
Diffstat (limited to 'projects')
-rw-r--r-- | projects/json-sanitizer/DenylistFuzzer.java | 25 | ||||
-rw-r--r-- | projects/json-sanitizer/IdempotenceFuzzer.java | 16 | ||||
-rw-r--r-- | projects/json-sanitizer/ValidJsonFuzzer.java | 9 |
3 files changed, 30 insertions, 20 deletions
diff --git a/projects/json-sanitizer/DenylistFuzzer.java b/projects/json-sanitizer/DenylistFuzzer.java index 9f810224a..4e73cfcb7 100644 --- a/projects/json-sanitizer/DenylistFuzzer.java +++ b/projects/json-sanitizer/DenylistFuzzer.java @@ -15,7 +15,8 @@ //////////////////////////////////////////////////////////////////////////////// import com.code_intelligence.jazzer.api.FuzzedDataProvider; - +import com.code_intelligence.jazzer.api.FuzzerSecurityIssueHigh; +import com.code_intelligence.jazzer.api.FuzzerSecurityIssueMedium; import com.google.json.JsonSanitizer; public class DenylistFuzzer { @@ -29,12 +30,20 @@ public class DenylistFuzzer { // exceeded. return; } - // See https://github.com/OWASP/json-sanitizer#output. - if (output.contains("</script") || output.contains("<script") - || output.contains("<!--") || output.contains("]]>")) { - System.err.println("input : " + input); - System.err.println("output: " + output); - throw new IllegalStateException("Output contains forbidden substring"); - } + + // Check for forbidden substrings. As these would enable Cross-Site + // Scripting, treat every finding as a high severity vulnerability. + assert !output.contains("</script") + : new FuzzerSecurityIssueHigh("Output contains </script"); + assert !output.contains("]]>") + : new FuzzerSecurityIssueHigh("Output contains ]]>"); + + // Check for more forbidden substrings. As these would not directly enable + // Cross-Site Scripting in general, but may impact script execution on the + // embedding page, treat each finding as a medium severity vulnerability. + assert !output.contains("<script") + : new FuzzerSecurityIssueMedium("Output contains <script"); + assert !output.contains("<!--") + : new FuzzerSecurityIssueMedium("Output contains <!--"); } } diff --git a/projects/json-sanitizer/IdempotenceFuzzer.java b/projects/json-sanitizer/IdempotenceFuzzer.java index 833ec3f0a..a42c91af9 100644 --- a/projects/json-sanitizer/IdempotenceFuzzer.java +++ b/projects/json-sanitizer/IdempotenceFuzzer.java @@ -21,20 +21,18 @@ import com.google.json.JsonSanitizer; public class IdempotenceFuzzer { public static void fuzzerTestOneInput(FuzzedDataProvider data) { String input = data.consumeRemainingAsString(); - String output1; + String output; try { - output1 = JsonSanitizer.sanitize(input, 10); + output = JsonSanitizer.sanitize(input, 10); } catch (ArrayIndexOutOfBoundsException e) { // ArrayIndexOutOfBoundsException is expected if nesting depth is // exceeded. return; } - String output2 = JsonSanitizer.sanitize(output1, 10); - if (!output1.equals(output2)) { - System.err.println("input : " + input); - System.err.println("output1: " + output1); - System.err.println("output2: " + output2); - throw new IllegalStateException("Non-idempotence detected"); - } + + // Ensure that sanitizing twice does not give different output + // (idempotence). Since failure to be idempotent is not a security issue in + // itself, fail with a regular AssertionError. + assert JsonSanitizer.sanitize(output).equals(output) : "Not idempotent"; } } diff --git a/projects/json-sanitizer/ValidJsonFuzzer.java b/projects/json-sanitizer/ValidJsonFuzzer.java index 7430b9c92..c8fbe0386 100644 --- a/projects/json-sanitizer/ValidJsonFuzzer.java +++ b/projects/json-sanitizer/ValidJsonFuzzer.java @@ -15,6 +15,7 @@ //////////////////////////////////////////////////////////////////////////////// import com.code_intelligence.jazzer.api.FuzzedDataProvider; +import com.code_intelligence.jazzer.api.FuzzerSecurityIssueLow; import com.google.gson.Gson; import com.google.gson.JsonElement; @@ -33,12 +34,14 @@ public class ValidJsonFuzzer { // exceeded. return; } + + // Check that the output is valid JSON. Invalid JSON may crash other parts + // of the application that trust the output of the sanitizer. try { + Gson gson = new Gson(); gson.fromJson(output, JsonElement.class); } catch (Exception e) { - System.err.println("input : " + input); - System.err.println("output : " + output); - throw e; + throw new FuzzerSecurityIssueLow("Output is invalid JSON", e); } } } |