Java: add IOUtils.toByteArray summaries

[jcogs33]

This PR adds a manual summary for IOUtils.toByteArray(InputStream) in order to cover CVE-2020-15232. This method was previously a df-generated neutral. Note that similar overloads are already summaries.

It also adds manual models for all other overloads of IOUtils.toByteArray (see #16964 (comment)).

[github-actions]

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

• Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,562,124,105,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,570,124,105,,,,,15
-    Totals,,310,25135,2598,367,16,128,33,1,409
+    Totals,,310,25143,2598,367,16,128,33,1,409

• Changes to framework-coverage-java.csv:

- org.apache.commons.io,124,,562,,,,,,,,,4,,,,,,,,,,,,,,,105,,,,,,,,,15,,,,,,,,,,,,,,,548,14
+ org.apache.commons.io,124,,570,,,,,,,,,4,,,,,,,,,,,,,,,105,,,,,,,,,15,,,,,,,,,,,,,,,556,14

[owen-mc]

Actually, now I've thought about it, I think you should add manual models for all the overloads of this method. Future runs of the dataflow model generator could generate different models for them, and you've manually verified that they should have summaries.

[jcogs33]

@owen-mc Thanks for the review! I've added the rest of the overloads in cd82ada. Note that IOUtils.toByteArray(URI) and IOUtils.toByteArray(URL) also have request-forgery sinks, so they may create overlapping paths for queries that use those sinks. Let me know if you think that is an issue and if I should remove those two?

[owen-mc]

I don't think it's an issue. If we start seeing that kind of problem we can deal with it. I've made queries stop at the first sink before.

[owen-mc]

If you have to push again to make CI pass, you could also delete the generated models for the APIs you added manual models for. It's not necessary, but it's just slightly less confusing if someone searches and finds both models.

[jcogs33]

@owen-mc Do you feel strongly that the generated models should be deleted? It had occurred to me earlier, but I didn't delete them since the generated file says "DO NOT EDIT". 🙂

[owen-mc]

When I add manual models, I delete the generated models for the same APIs. They won't be used (as the manual models take precedence), and if the manual models had existed before they wouldn't have been created (we don't generate models for APIs which have manual models). I assume that generated files say "do not edit" because if they get re-generated then the edits will be lost, but in this case I'm actually pre-empting what would happen if they were re-generated (ignoring the fact that we'd get slightly different models). Also, it's a bit confusing to have two models for the same thing, where one of them has no effect.

I don't feel that strongly (I wouldn't block a PR on it, or change it after the PR was merged), but I try to remember to do it.

[jcogs33]

I'm not going to delete them in this case so I can get this PR merged before there are any more conflicts with the test files 😬, but I'll keep it in mind for the future.

[michaelnebel]

A pre-requisite for getting the model generator to produce these models is https://github.com/github/codeql-team/issues/713
Many of the toByteArray overloads uses a copy method (with a wrapper implementation)`

public static void copy(final Reader reader, final OutputStream output, final Charset outputCharset)
            throws IOException {
        final OutputStreamWriter writer = new OutputStreamWriter(output, Charsets.toCharset(outputCharset));
        copy(reader, writer);
        // XXX Unless anyone is planning on rewriting OutputStreamWriter,
        // we have to flush here.
        writer.flush();
    }