[Java] Support local taint tracking through JDK Collections lambdas

[JLLeitschuh]

Adds support for local taint tracking through common simple JDK collections functional APIs like forEach and forEachRemaining.

[JLLeitschuh]

I notice that entry.getKey() isn't considered tainted. Should the same be true here?
If so, is there a good way to know if the key is tainted by user supplied data or not to decide whether or not to filter it out?

[aschackmull]

FYI: We have plans to support such lambda-based APIs in the hopefully not too distant future, which will obsolete this work. (I was about to say besides the tests, but the collection-based tests are also a bit up in the air with the current rework of collection-modelling - we aim to make collection-flow more precise and value-flow-based rather than being taint-steps).

[JLLeitschuh]

Hey @aschackmull,

Totally fine if this work get's obsoleted in the near future. I just need it to work in the near-term as a stop-gap until full support for lambda based stuff is rolled out.

In particular this is why I've been opening all of these PRs.

• [Java] Fix Kryo FP & Kryo 5 Support #5843
• [Java] Add taint tracking through Jackson deserialization #5814
• Java: Simple support for Ratpack HTTP Framework #4991
• [Java] Add support for com.google.common.base.MoreObjects#firstNonNull #5824
• [Java] Support local taint tracking through JDK Collections lambdas #5871

The data flow that I'm trying to support for our application is the following data flow:

    private static Promise<Form> parseToForm(Context ctx, Class<?> type) {
        return ctx.getRequest().getBody() // `getBody` is the source of user-supplied 'taint'. #4991
            .map(data -> {
                try {
                    return ctx.parse(data, Form.form()); // Taint flow. #4991
                } catch (Exception e) {
                    String msg = "Unable to parse form data while binding type " + type.getCanonicalName() + " [formData: " + data.getText() + "]";
                    LOGGER.error(msg, e);
                    throw new RequestBindException(msg, e);
                }
            });
    }

    public <T> Promise<T> bindForm(Context ctx, Class<T> type) {
        return bindForm(ctx, type, true, Action.noop());
    }

    public <T> Promise<T> bindForm(Context ctx, Class<T> type, Action<? super ImmutableMap.Builder<String, Object>> defaults) {
        return bindForm(ctx, type, true, defaults);
    }

    private <T> Promise<T> bindForm(Context ctx, Class<T> type, boolean requireCsrfProtection, Action<? super ImmutableMap.Builder<String, Object>> defaults) {
        return parseToForm(ctx, type)
            .map(form -> {
                ObjectNode input = toObjectNode(form, defaults, s -> false);
                Map<String, List<UploadedFile>> filesMap = form.files().getAll();
                filesMap.forEach((name, files) -> {
                    ArrayNode array = input.putArray(name);
                    files.forEach(f -> array.add(new POJONode(new UploadedFileWrapper(f))));
                });
                return bind(ctx, input, type, requireCsrfProtection);
            });
    }

    private ObjectNode toObjectNode(MultiValueMap<String, String> params, Action<? super ImmutableMap.Builder<String, Object>> defaults, Predicate<String> paramFilter) throws Exception {
        Map<String, Object> mergedParams = new HashMap<>(defaults.with(ImmutableMap.builder()).build());
        filterAndMerge(params, mergedParams, paramFilter);
        throwOnIllegalParams(mergedParams);
        return objectMapper.valueToTree(mergedParams); // The reason for #5814
    }

    private static void filterAndMerge(MultiValueMap<String, String> params, Map<String, Object> defaults, Predicate<String> filter) {
        params.asMultimap().asMap().forEach((name, values) -> { // The reason for this PR!
            if (!isEmptyAndHasDefault(name, values, defaults) && !filter.test(name)) {
                defaults.put(name, extractSingleValueIfPossible(values));
            }
        });
    }

    private <T> T bind(Context ctx, JsonNode input, Class<T> type, boolean requireCsrfProtection) {
        // ...

        T value;
        try {
            value = objectMapper.convertValue(input, type); // The reason for #5814
        } catch (Exception e) {
            throw new RequestBindException("Failed to convert input to " + type.getName(), e);
        }

        /// ...
        return value;
    }

What I'm trying to communicate is that. Although I don't really care whether or not this change that I've made in this PR lasts long-term, it will at least provide a stop-gap until proper support is added.

Under that context, would you be willing to review this PR, or would you still like me to approach this from a different perspective?

If, as a user, I could configure custom AdditionalTaintStep specific to my application that all of the existing CodeQL queries could run against, I'd do that. Unfortunately, that doesn't seem to be officially supported at this time.

[Marcono1234]

Maybe it would be good to use DataFlow::getInstanceArgument(Call) as suggested in #5814 (comment)

[JLLeitschuh]

Replaced by: #6917