Remove 2020 sinks from TaintedPath.ql

This commit is contained in:
Esben Sparre Andreasen
2020-10-06 13:11:04 +02:00
committed by Ian Wright
parent dca03d7b5d
commit 24017151c7
4 changed files with 18 additions and 506 deletions

View File

@@ -927,28 +927,6 @@ module Express {
override string getCredentialsKind() { result = kind }
}
/** A call to `response.sendFile`, considered as a file system access. */
private class ResponseSendFileAsFileSystemAccess extends FileSystemReadAccess,
DataFlow::MethodCallNode {
ResponseSendFileAsFileSystemAccess() {
exists(string name | name = "sendFile" or name = "sendfile" |
this.calls(any(ResponseExpr res).flow(), name)
)
}
override DataFlow::Node getADataNode() { none() }
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getRootPathArgument() {
result = this.(DataFlow::CallNode).getOptionArgument(1, "root")
}
override predicate isUpwardNavigationRejected(DataFlow::Node argument) {
argument = this.getAPathArgument()
}
}
/**
* A function that flows to a route setup.
*/

View File

@@ -4,23 +4,6 @@
import javascript
/**
* A call that can produce a file name.
*/
abstract private class FileNameProducer extends DataFlow::Node {
/**
* Gets a file name produced by this producer.
*/
abstract DataFlow::Node getAFileName();
}
/**
* A node that contains a file name, and is produced by a `ProducesFileNames`.
*/
private class ProducedFileName extends FileNameSource {
ProducedFileName() { this = any(FileNameProducer producer).getAFileName() }
}
/**
* A file name from the `walk-sync` library.
*/
@@ -143,341 +126,3 @@ private DataFlow::Node fastGlobFileNameSource(DataFlow::TypeTracker t) {
private class FastGlobFileNameSource extends FileNameSource {
FastGlobFileNameSource() { this = fastGlobFileNameSource(DataFlow::TypeTracker::end()) }
}
/**
* Classes and predicates for modeling the `fstream` library (https://www.npmjs.com/package/fstream).
*/
private module FStream {
/**
* Gets a reference to a method in the `fstream` library.
*/
private DataFlow::SourceNode getAnFStreamProperty(boolean writer) {
exists(DataFlow::SourceNode mod, string readOrWrite, string subMod |
mod = DataFlow::moduleImport("fstream") and
(
readOrWrite = "Reader" and writer = false
or
readOrWrite = "Writer" and writer = true
) and
subMod = ["File", "Dir", "Link", "Proxy"]
|
result = mod.getAPropertyRead(readOrWrite) or
result = mod.getAPropertyRead(readOrWrite).getAPropertyRead(subMod) or
result = mod.getAPropertyRead(subMod).getAPropertyRead(readOrWrite)
)
}
/**
* An invocation of a method defined in the `fstream` library.
*/
private class FStream extends FileSystemAccess, DataFlow::InvokeNode {
boolean writer;
FStream() { this = getAnFStreamProperty(writer).getAnInvocation() }
override DataFlow::Node getAPathArgument() {
result = this.getOptionArgument(0, "path")
or
not exists(this.getOptionArgument(0, "path")) and
result = this.getArgument(0)
}
}
/**
* An invocation of an `fstream` method that writes to a file.
*/
private class FStreamWriter extends FileSystemWriteAccess, FStream {
FStreamWriter() { writer = true }
override DataFlow::Node getADataNode() { none() }
}
/**
* An invocation of an `fstream` method that reads a file.
*/
private class FStreamReader extends FileSystemReadAccess, FStream {
FStreamReader() { writer = false }
override DataFlow::Node getADataNode() { none() }
}
}
/**
* A call to the library `write-file-atomic`.
*/
private class WriteFileAtomic extends FileSystemWriteAccess, DataFlow::CallNode {
WriteFileAtomic() {
this = DataFlow::moduleImport("write-file-atomic").getACall()
or
this = DataFlow::moduleMember("write-file-atomic", "sync").getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getADataNode() { result = this.getArgument(1) }
}
/**
* A call to the library `recursive-readdir`.
*/
private class RecursiveReadDir extends FileSystemAccess, FileNameProducer, DataFlow::CallNode {
RecursiveReadDir() { this = DataFlow::moduleImport("recursive-readdir").getACall() }
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getAFileName() {
result = this.trackFileSource(DataFlow::TypeTracker::end())
}
private DataFlow::SourceNode trackFileSource(DataFlow::TypeTracker t) {
t.start() and result = this.getCallback([1 .. 2]).getParameter(1)
or
t.startInPromise() and not exists(this.getCallback([1 .. 2])) and result = this
or
// Tracking out of a promise
exists(DataFlow::TypeTracker t2 |
result = PromiseTypeTracking::promiseStep(this.trackFileSource(t2), t, t2)
)
}
}
/**
* Classes and predicates for modeling the `jsonfile` library (https://www.npmjs.com/package/jsonfile).
*/
private module JSONFile {
/**
* A reader for JSON files.
*/
class JSONFileReader extends FileSystemReadAccess, DataFlow::CallNode {
JSONFileReader() {
this =
DataFlow::moduleMember("jsonfile", any(string s | s = "readFile" or s = "readFileSync"))
.getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getADataNode() { result = this.trackRead(DataFlow::TypeTracker::end()) }
private DataFlow::SourceNode trackRead(DataFlow::TypeTracker t) {
this.getCalleeName() = "readFile" and
(
t.start() and result = this.getCallback([1 .. 2]).getParameter(1)
or
t.startInPromise() and not exists(this.getCallback([1 .. 2])) and result = this
)
or
t.start() and
this.getCalleeName() = "readFileSync" and
result = this
or
// Tracking out of a promise
exists(DataFlow::TypeTracker t2 |
result = PromiseTypeTracking::promiseStep(this.trackRead(t2), t, t2)
)
}
}
/**
* A writer for JSON files.
*/
class JSONFileWriter extends FileSystemWriteAccess, DataFlow::CallNode {
JSONFileWriter() {
this =
DataFlow::moduleMember("jsonfile", any(string s | s = "writeFile" or s = "writeFileSync"))
.getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getADataNode() { result = this.getArgument(1) }
}
}
/**
* A call to the library `load-json-file`.
*/
private class LoadJsonFile extends FileSystemReadAccess, DataFlow::CallNode {
LoadJsonFile() {
this = DataFlow::moduleImport("load-json-file").getACall()
or
this = DataFlow::moduleMember("load-json-file", "sync").getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getADataNode() { result = this.trackRead(DataFlow::TypeTracker::end()) }
private DataFlow::SourceNode trackRead(DataFlow::TypeTracker t) {
this.getCalleeName() = "sync" and t.start() and result = this
or
not this.getCalleeName() = "sync" and t.startInPromise() and result = this
or
// Tracking out of a promise
exists(DataFlow::TypeTracker t2 |
result = PromiseTypeTracking::promiseStep(this.trackRead(t2), t, t2)
)
}
}
/**
* A call to the library `write-json-file`.
*/
private class WriteJsonFile extends FileSystemWriteAccess, DataFlow::CallNode {
WriteJsonFile() {
this = DataFlow::moduleImport("write-json-file").getACall()
or
this = DataFlow::moduleMember("write-json-file", "sync").getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getADataNode() { result = this.getArgument(1) }
}
/**
* A call to the library `walkdir`.
*/
private class WalkDir extends FileNameProducer, FileSystemAccess, DataFlow::CallNode {
WalkDir() {
this = DataFlow::moduleImport("walkdir").getACall()
or
this = DataFlow::moduleMember("walkdir", "sync").getACall()
or
this = DataFlow::moduleMember("walkdir", "async").getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getAFileName() {
result = this.trackFileSource(DataFlow::TypeTracker::end())
}
private DataFlow::SourceNode trackFileSource(DataFlow::TypeTracker t) {
not this.getCalleeName() = any(string s | s = "sync" or s = "async") and
t.start() and
(
result = this.getCallback(this.getNumArgument() - 1).getParameter(0)
or
result = this.getAMethodCall(EventEmitter::on()).getCallback(1).getParameter(0)
)
or
t.start() and this.getCalleeName() = "sync" and result = this
or
t.startInPromise() and this.getCalleeName() = "async" and result = this
or
// Tracking out of a promise
exists(DataFlow::TypeTracker t2 |
result = PromiseTypeTracking::promiseStep(this.trackFileSource(t2), t, t2)
)
}
}
/**
* A call to the library `globule`.
*/
private class Globule extends FileNameProducer, FileSystemAccess, DataFlow::CallNode {
Globule() {
this = DataFlow::moduleMember("globule", "find").getACall()
or
this = DataFlow::moduleMember("globule", "match").getACall()
or
this = DataFlow::moduleMember("globule", "isMatch").getACall()
or
this = DataFlow::moduleMember("globule", "mapping").getACall()
or
this = DataFlow::moduleMember("globule", "findMapping").getACall()
}
override DataFlow::Node getAPathArgument() {
(this.getCalleeName() = "match" or this.getCalleeName() = "isMatch") and
result = this.getArgument(1)
or
this.getCalleeName() = "mapping" and
(
result = this.getAnArgument() and
not exists(result.getALocalSource().getAPropertyWrite("src"))
or
result = this.getAnArgument().getALocalSource().getAPropertyWrite("src").getRhs()
)
}
override DataFlow::Node getAFileName() {
result = this and
(
this.getCalleeName() = "find" or
this.getCalleeName() = "match" or
this.getCalleeName() = "findMapping" or
this.getCalleeName() = "mapping"
)
}
}
/**
* A file system access made by a NodeJS library.
* This class models multiple NodeJS libraries that access files.
*/
private class LibraryAccess extends FileSystemAccess, DataFlow::InvokeNode {
int pathArgument; // The index of the path argument.
LibraryAccess() {
pathArgument = 0 and
(
this = DataFlow::moduleImport("path-exists").getACall()
or
this = DataFlow::moduleImport("rimraf").getACall()
or
this = DataFlow::moduleImport("readdirp").getACall()
or
this = DataFlow::moduleImport("walker").getACall()
or
this =
DataFlow::moduleMember("node-dir",
["readFiles", "readFilesStream", "files", "promiseFiles", "subdirs", "paths"]).getACall()
)
or
pathArgument = 0 and
this =
DataFlow::moduleMember("vinyl-fs", any(string s | s = "src" or s = "dest" or s = "symlink"))
.getACall()
or
pathArgument = [0 .. 1] and
(
this = DataFlow::moduleImport("ncp").getACall() or
this = DataFlow::moduleMember("ncp", "ncp").getACall()
)
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArgument) }
}
/**
* A call to the library [`chokidar`](https://www.npmjs.com/package/chokidar), where a call to `on` receives file names.
*/
class Chokidar extends FileNameProducer, FileSystemAccess, API::CallNode {
Chokidar() { this = API::moduleImport("chokidar").getMember("watch").getACall() }
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
override DataFlow::Node getAFileName() {
exists(DataFlow::CallNode onCall, int pathIndex |
onCall = this.getAChainedMethodCall("on") and
if onCall.getArgument(0).mayHaveStringValue("all") then pathIndex = 1 else pathIndex = 0
|
result = onCall.getCallback(1).getParameter(pathIndex)
)
}
}
/**
* A call to the [`mkdirp`](https://www.npmjs.com/package/mkdirp) library.
*/
private class Mkdirp extends FileSystemAccess, API::CallNode {
Mkdirp() {
this = API::moduleImport("mkdirp").getACall()
or
this = API::moduleImport("mkdirp").getMember("sync").getACall()
}
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
}

View File

@@ -496,53 +496,8 @@ module NodeJSLib {
* A member `member` from module `fs` or its drop-in replacements `graceful-fs`, `fs-extra`, `original-fs`.
*/
DataFlow::SourceNode moduleMember(string member) {
result = fsModule(DataFlow::TypeTracker::end()).getAPropertyRead(member)
}
private DataFlow::SourceNode fsModule(DataFlow::TypeTracker t) {
exists(string moduleName |
moduleName = ["mz/fs", "original-fs", "fs-extra", "graceful-fs", "fs"]
|
result = DataFlow::moduleImport(moduleName)
or
// extra support for flexible names
result.asExpr().(Require).getArgument(0).mayHaveStringValue(moduleName)
) and
t.start()
or
t.start() and
result = DataFlow::moduleMember("fs", "promises")
or
exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = fsModule(t2) |
result = pred.track(t2, t)
or
t.continue() = t2 and
exists(Promisify::PromisifyAllCall promisifyAllCall |
result = promisifyAllCall and
pred.flowsTo(promisifyAllCall.getArgument(0))
)
or
// const fs = require('fs');
// let fs_copy = methods.reduce((obj, method) => {
// obj[method] = fs[method];
// return obj;
// }, {});
t.continue() = t2 and
exists(
DataFlow::MethodCallNode call, DataFlow::ParameterNode obj, DataFlow::SourceNode method
|
call.getMethodName() = "reduce" and
result = call and
obj = call.getABoundCallbackParameter(0, 0) and
obj.flowsTo(any(DataFlow::FunctionNode f).getAReturn()) and
exists(DataFlow::PropWrite write, DataFlow::PropRead read |
write = obj.getAPropertyWrite() and
method.flowsToExpr(write.getPropertyNameExpr()) and
method.flowsToExpr(read.getPropertyNameExpr()) and
read.getBase().getALocalSource() = fsModule(t2) and
write.getRhs() = maybePromisified(read)
)
)
exists(string moduleName | moduleName = ["fs-extra", "graceful-fs", "fs"] |
result = DataFlow::moduleMember(moduleName, member)
)
}
}
@@ -553,7 +508,7 @@ module NodeJSLib {
private class NodeJSFileSystemAccess extends FileSystemAccess, DataFlow::CallNode {
string methodName;
NodeJSFileSystemAccess() { this = maybePromisified(FS::moduleMember(methodName)).getACall() }
NodeJSFileSystemAccess() { this = FS::moduleMember(methodName).getACall() }
/**
* Gets the name of the called method.

View File

@@ -55,7 +55,7 @@ module TaintedPath {
* There are currently four flow labels, representing the different combinations of
* normalization and absoluteness.
*/
abstract class PosixPath extends DataFlow::FlowLabel {
class PosixPath extends DataFlow::FlowLabel {
Normalization normalization;
Relativeness relativeness;
@@ -113,7 +113,7 @@ module TaintedPath {
/**
* A flow label representing an array of path elements that may include "..".
*/
abstract class SplitPath extends DataFlow::FlowLabel {
class SplitPath extends DataFlow::FlowLabel {
SplitPath() { this = "splitPath" }
}
}
@@ -218,12 +218,12 @@ module TaintedPath {
output = this
or
// non-global replace or replace of something other than /\.\./g, /[/]/g, or /[\.]/g.
this instanceof StringReplaceCall and
input = this.getReceiver() and
this.getCalleeName() = "replace" and
input = getReceiver() and
output = this and
not exists(RegExpLiteral literal, RegExpTerm term |
this.(StringReplaceCall).getRegExp().asExpr() = literal and
this.(StringReplaceCall).isGlobal() and
getArgument(0).getALocalSource().asExpr() = literal and
literal.isGlobal() and
literal.getRoot() = term
|
term.getAMatchedString() = "/" or
@@ -247,15 +247,16 @@ module TaintedPath {
/**
* A call that removes all instances of "../" in the prefix of the string.
*/
class DotDotSlashPrefixRemovingReplace extends StringReplaceCall {
class DotDotSlashPrefixRemovingReplace extends DataFlow::CallNode {
DataFlow::Node input;
DataFlow::Node output;
DotDotSlashPrefixRemovingReplace() {
input = this.getReceiver() and
this.getCalleeName() = "replace" and
input = getReceiver() and
output = this and
exists(RegExpLiteral literal, RegExpTerm term |
this.getRegExp().asExpr() = literal and
getArgument(0).getALocalSource().asExpr() = literal and
(term instanceof RegExpStar or term instanceof RegExpPlus) and
term.getChild(0) = getADotDotSlashMatcher()
|
@@ -297,16 +298,17 @@ module TaintedPath {
/**
* A call that removes all "." or ".." from a path, without also removing all forward slashes.
*/
class DotRemovingReplaceCall extends StringReplaceCall {
class DotRemovingReplaceCall extends DataFlow::CallNode {
DataFlow::Node input;
DataFlow::Node output;
DotRemovingReplaceCall() {
input = this.getReceiver() and
this.getCalleeName() = "replace" and
input = getReceiver() and
output = this and
this.isGlobal() and
exists(RegExpLiteral literal, RegExpTerm term |
this.getRegExp().asExpr() = literal and
getArgument(0).getALocalSource().asExpr() = literal and
literal.isGlobal() and
literal.getRoot() = term and
not term.getAMatchedString() = "/"
|
@@ -648,74 +650,6 @@ module TaintedPath {
AngularJSTemplateUrlSink() { this = any(AngularJS::CustomDirective d).getMember("templateUrl") }
}
/**
* The path argument of a [send](https://www.npmjs.com/package/send) call, viewed as a sink.
*/
class SendPathSink extends Sink, DataFlow::ValueNode {
SendPathSink() { this = DataFlow::moduleImport("send").getACall().getArgument(1) }
}
/**
* A path argument given to a `Page` in puppeteer, specifying where a pdf/screenshot should be saved.
*/
private class PuppeteerPath extends TaintedPath::Sink {
PuppeteerPath() {
this =
Puppeteer::page()
.getMember(["pdf", "screenshot"])
.getParameter(0)
.getMember("path")
.getARhs()
}
}
/**
* An argument given to the `prettier` library specifying the location of a config file.
*/
private class PrettierFileSink extends TaintedPath::Sink {
PrettierFileSink() {
this =
API::moduleImport("prettier")
.getMember(["resolveConfig", "resolveConfigFile", "getFileInfo"])
.getACall()
.getArgument(0)
or
this =
API::moduleImport("prettier")
.getMember("resolveConfig")
.getACall()
.getParameter(1)
.getMember("config")
.getARhs()
}
}
/**
* The `cwd` option for the `read-pkg` library.
*/
private class ReadPkgCwdSink extends TaintedPath::Sink {
ReadPkgCwdSink() {
this =
API::moduleImport("read-pkg")
.getMember(["readPackageAsync", "readPackageSync"])
.getParameter(0)
.getMember("cwd")
.getARhs()
}
}
/**
* The `cwd` option to a shell execution.
*/
private class ShellCwdSink extends TaintedPath::Sink {
ShellCwdSink() {
exists(SystemCommandExecution sys, API::Node opts |
opts.getARhs() = sys.getOptionsArg() and // assuming that an API::Node exists here.
this = opts.getMember("cwd").getARhs()
)
}
}
/**
* Holds if there is a step `src -> dst` mapping `srclabel` to `dstlabel` relevant for path traversal vulnerabilities.
*/