[JS]: New command execution sinks(Execa, shelljs and dynamic import)

[am0o0]

Query PR

github/codeql#14291
github/codeql#14293
github/codeql#14294

Language

Javascript

CVE(s) ID list

• CVE-2023-3224

CWE

CWE-078

Report

Execa package before version 5 has already been modeled but newer versions up to 8 have many new APIs that I've implemented now.
Shelljs package also has a piping feature which I've updated the current shelljs module to support piping too.
Also, dynamic import in nodejs support URLs starts with data: which is dangerous.
There is another nodejs API that accepts the data: URL which is:

const {Worker} = require('node:worker_threads');
new Worker(new URL('data:text/javascript,console.log("hello!");'))

but it needs to be a URL Type as input, not any string value that starts with data:, I'm not sure what is the best way to implement it.

Are you planning to discuss this vulnerability submission publicly? (Blog Post, social networks, etc).

• Yes
• No

Blog post link

No response

[ghsecuritylab]

Your submission is now in status Test run.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

[am0o0]

Hi, as suggested by Erik Krogh Kristensen, I made three separate pull requests for each package change.
you can still use the previous pull for submission evaluation.

[pwntester]

Thanks for yet another submission @amammad !

My understanding is that the dynamic import will only be vulnerable if an attacker controls the entire path passed to the import() call or at least the begining of it. Could you add a sanitizer that excludes those cases where the path is prefixed by a non-user controlled prefix?

Here is an example used by the DOM XSS query

[am0o0]

@pwntester thanks for the previous bounties and Also thanks for your help.

I'm trying to implement this but idk how to work with isLabeledBarrier it can take a little bit of my time to solve this.

[pwntester]

Hi, You dont need to use FlowLabels for this query. You can write a regular sanitizer that do not uses flow labels and then use the StringOps::ConcatenationRoot to check any node in the path is prefixing any data. For example, check this one

[am0o0]

Hi @pwntester I created a new experimental query file for dynamic import and Worker sinks, there is a little problem with it that I'm going to put comment about it to the PR

[ghsecuritylab]

Your submission is now in status Query review.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

[ghsecuritylab]

Your submission is now in status Final decision.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

[ghsecuritylab]

Your submission is now in status Pay.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

[xcorail]

Created Hackerone report 2513300 for bounty 579597 : [788] [JS]: New command execution sinks(Execa, shelljs and dynamic import)

[ghsecuritylab]

Your submission is now in status Closed.

For information, the evaluation workflow is the following:
Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed