hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-20 14:17:11 +02:00
Code Issues Packages Projects Releases Wiki Activity

Remove 2020 sinks from TaintedPath.ql

Browse Source
This commit is contained in:
Esben Sparre Andreasen
2020-10-06 13:11:04 +02:00
committed by Henry Mercer
parent 7fef1a8817
commit 9832847921
4 changed files with 15 additions and 392 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
javascript/ql/src/semmle/javascript/frameworks/Express.qll
View File

@@ -861,28 +861,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" |
calls(any(ResponseExpr res).flow(), name)
)
}
override DataFlow::Node getADataNode() { none() }
override DataFlow::Node getAPathArgument() { result = getArgument(0) }
override DataFlow::Node getRootPathArgument() {
result = this.(DataFlow::CallNode).getOptionArgument(1, "root")
}
override predicate isUpwardNavigationRejected(DataFlow::Node argument) {
argument = getAPathArgument()
}
}
/**
* A function that flows to a route setup.
*/

325
javascript/ql/src/semmle/javascript/frameworks/Files.qll
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.
*/
@@ -144,311 +127,3 @@ private class FastGlobFileNameSource extends FileNameSource {
FastGlobFileNameSource() { this = fastGlobFileNameSource(DataFlow::TypeTracker::end()) }
}
/**
* Classes and predicates for modelling 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" or subMod = "Dir" or subMod = "Link" or subMod = "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 = getOptionArgument(0, "path")
or
not exists(getOptionArgument(0, "path")) and
result = 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 = getArgument(0) }
override DataFlow::Node getADataNode() { result = 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 = getArgument(0) }
override DataFlow::Node getAFileName() { result = trackFileSource(DataFlow::TypeTracker::end()) }
private DataFlow::SourceNode trackFileSource(DataFlow::TypeTracker t) {
t.start() and result = getCallback([1 .. 2]).getParameter(1)
or
t.startInPromise() and not exists(getCallback([1 .. 2])) and result = this
or
// Tracking out of a promise
exists(DataFlow::TypeTracker t2 |
result = PromiseTypeTracking::promiseStep(trackFileSource(t2), t, t2)
)
}
}
/**
* Classes and predicates for modelling 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 = getArgument(0) }
override DataFlow::Node getADataNode() { result = trackRead(DataFlow::TypeTracker::end()) }
private DataFlow::SourceNode trackRead(DataFlow::TypeTracker t) {
this.getCalleeName() = "readFile" and
(
t.start() and result = getCallback([1 .. 2]).getParameter(1)
or
t.startInPromise() and not exists(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(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 = getArgument(0) }
override DataFlow::Node getADataNode() { result = 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 = getArgument(0) }
override DataFlow::Node getADataNode() { result = 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(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 = getArgument(0) }
override DataFlow::Node getADataNode() { result = 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 = getArgument(0) }
override DataFlow::Node getAFileName() { result = 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 = getCallback(getNumArgument() - 1).getParameter(0)
or
result = 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(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 = getArgument(1)
or
this.getCalleeName() = "mapping" and
(
result = getAnArgument() and not exists(result.getALocalSource().getAPropertyWrite("src"))
or
result = 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",
any(string s |
s = "readFiles" or
s = "readFilesStream" or
s = "files" or
s = "promiseFiles" or
s = "subdirs" or
s = "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 = getArgument(pathArgument) }
}

31
javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll
View File

@@ -450,33 +450,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
exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = fsModule(t2) |
result = pred.track(t2, t)
or
t.continue() = t2 and
exists(DataFlow::CallNode promisifyAllCall |
result = promisifyAllCall and
pred.flowsTo(promisifyAllCall.getArgument(0)) and
promisifyAllCall =
[
DataFlow::moduleMember("bluebird", "promisifyAll"),
DataFlow::moduleImport("util-promisifyall")
].getACall()
)
exists(string moduleName | moduleName = ["fs-extra", "graceful-fs", "fs"] |
result = DataFlow::moduleMember(moduleName, member)
)
}
}
@@ -487,7 +462,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.

29
javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
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
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() {
this.getCalleeName() = "replace" and
input = getReceiver() and
output = this and
exists(RegExpLiteral literal, RegExpTerm term |
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() {
this.getCalleeName() = "replace" and
input = getReceiver() and
output = this and
isGlobal() and
exists(RegExpLiteral literal, RegExpTerm term |
getRegExp().asExpr() = literal and
getArgument(0).getALocalSource().asExpr() = literal and
literal.isGlobal() and
literal.getRoot() = term and
not term.getAMatchedString() = "/"
|
@@ -624,13 +626,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) }
}
/**
* Holds if there is a step `src -> dst` mapping `srclabel` to `dstlabel` relevant for path traversal vulnerabilities.
*/