diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Express.qll b/javascript/ql/lib/semmle/javascript/frameworks/Express.qll index ca9a151e3c6..51346599973 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Express.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Express.qll @@ -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. */ diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Files.qll b/javascript/ql/lib/semmle/javascript/frameworks/Files.qll index e57ecd8cd4d..f0489c4e128 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Files.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Files.qll @@ -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) } -} diff --git a/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll index bf529913251..5f3aac27fd1 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll @@ -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. diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll index fcce7ce9aeb..85a56a05ef2 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll @@ -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. */