Skip to content

Commit

Permalink
JS: Model readline as a stdin threat-model source
Browse files Browse the repository at this point in the history
Technically not always true, but my assumption is that +90% of the time
that's what it will be used for, so while we could be more precise by
adding a taint-step from the `input` part of the construction, I'm not
sure it's worth it in this case.

Furthermore, doing so would break with the current way we model
threat-model sources, and how sources are generally modeled in JS... so
for a very pretty setup it would require changing all the other `file`
threat-model sources to start at the constructors such as
`fs.createReadStream()` and have taint-propagation steps towards the
actual use (like we do in Python)...

I couldn't see an easy path forwards for doing this while keeping the
Concepts integration, so I opted for the simpler solution here.
  • Loading branch information
RasmusWL committed Oct 31, 2024
1 parent eca8bf5 commit 61e60de
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ extensions:
- ['fs', 'Member[promises].Member[readFile].ReturnValue.Member[then].Argument[0].Parameter[0]', 'file']
- ['global', 'Member[process].Member[stdin].Member[read].ReturnValue', 'stdin']
- ['global', 'Member[process].Member[stdin].Member[on,addListener].WithStringArgument[0=data].Argument[1].Parameter[0]', 'stdin']
- ['readline', 'Member[createInterface].ReturnValue.Member[question].Argument[1].Parameter[0]', 'stdin']
- ['readline', 'Member[createInterface].ReturnValue.Member[on,addListener].WithStringArgument[0=line].Argument[1].Parameter[0]', 'stdin']
20 changes: 10 additions & 10 deletions javascript/ql/test/library-tests/threat-models/sources/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ SINK(data); // $ hasFlow
// using readline
const readline = require('readline');
const rl_file = readline.createInterface({
input: fs.createReadStream('file.txt') // $ MISSING: threat-source=file
input: fs.createReadStream('file.txt')
});
rl_file.on("line", (line) => {
SINK(line); // $ MISSING: hasFlow
rl_file.on("line", (line) => { // $ SPURIOUS: threat-source=stdin MISSING: threat-source=file
SINK(line); // $ hasFlow
});


Expand All @@ -104,17 +104,17 @@ SINK(stdin_line); // $ hasFlow
// Accessing stdin using readline
const readline = require('readline');
const rl_stdin = readline.createInterface({
input: process.stdin // $ MISSING: threat-source=stdin
input: process.stdin
});
rl_stdin.question('<question>', (answer) => {
SINK(answer); // $ MISSING: hasFlow
rl_stdin.question('<question>', (answer) => { // $ threat-source=stdin
SINK(answer); // $ hasFlow
});

function handler(answer) {
SINK(answer); // $ MISSING: hasFlow
function handler(answer) { // $ threat-source=stdin
SINK(answer); // $ hasFlow
}
rl_stdin.question('<question>', handler);

rl_stdin.on("line", (line) => {
SINK(line); // $ MISSING: hasFlow
rl_stdin.on("line", (line) => { // $ threat-source=stdin
SINK(line); // $ hasFlow
});

0 comments on commit 61e60de

Please sign in to comment.