JS: add js/path-injection-from-library-input query

[kaeluka]

This PR adds library inputs as taint sources. It is related to a current CVE that we did miss partially due to these sources missing [1]. However, as there's a lot of library inputs, it's important to discuss whether this should be merged or not.

I've started an LGTM experiment comparing the results of the main branch with those of adding this PR's changes and the changes of #8430. Find the results here: baseline / patched.

For example, the patched version (due to the new input sources) finds lots of results in alibaba/ice that are not found in the baseline. In my opinion, those are good results that I'd appreciate as a developer. But I have a gut feeling that the team might consider these too noisy. Discuss! :)

[1] - The other part is a missing taint step that I'll be contributing in another PR #8430.

[asgerf]

I used this LGTM run to show just the new alerts, which was easier to triage. After looking through these results I couldn't find any results that I would consider a TP 😕

The input parameter/property often has a name that indicates it is intended to be a path, such as cwd, filepath, or something that ends with file or path.

Sometimes the fact that the input is interpreted as a path is explained by a documentation comment, for example for pm2_home:

/**
*  @param {String}  [opts.pm2_home=[<paths.js>]] pm2 directory for log, pids, socket files
*/

If we're going to go ahead with this, the first thing to do is to avoid inputs that are clearly intended to be interpreted as paths.

Overall:

• Implement as a separate query.
• Adjust metadata in the new query (@precision warning) and make the description and alert messaging clearly indicate that this is library input, not user data.
• Use sanitizers to implement name filters for properties. For example opts.cwd could be a sanitizer. Such property accesses can occur arbitrarily far away from the source, so it's more effective than simply trying to filter out sources.

[kaeluka]

• Implement as a separate query.
• Adjust metadata in the new query (@precision warning) and make the description and alert messaging clearly indicate that this is library input, not user data.

That feedback was really valuable! I hadn't considered the option to create a new, lower precision query 🙈. Thank you 👍.
I believe I have found another CVE we have missed where the source is a library input. I'll look at some more candidate CVEs to see if there is a pattern. Then I'll adapt this PR to my learnings.

[kaeluka]

Converted the query to a standalone warning query; name filters still missing

[kaeluka]

I added one more commit that filters out sinks that are parameters that have an @param comment which mentions the parameter is a file/path/etc.

[erik-krogh]

I haven't taken a detailed look at the implementation/documentation yet, but I did find a few things that jumped out.

I tried to investigate all the new results from your evaluation, and I could categorize the projects as follows:

• documented/intentional behavior: 3
• test code: 2
• build-script: 1
• maybe a bit interesting: 1

That's not a great TP rate.

The interesting result I found follow the source -> path construction -> path sink-pattern that is also used by e.g. js/shell-command-constructed-from-input, so we might be able to use that to cut down on the FPs.

Also, you need to copy this code to cut down some other FPs.

[kaeluka]

rebased onto main because the "Check synchronized files / sync (pull_request)" test used to be broken, but has now been fixed in main.

[kaeluka]

Have gotten a lot of failed DCA runs on this and the work got side tracked by other tasks I was working on. Picking up again now.