hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-16 16:53:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #39 from asger-semmle/nodejslib-imports

Browse Source 
JavaScript: Use 'moduleMember' in NodeJSLib.qll for ES6-compatibility
This commit is contained in:
Max Schaefer
2018-08-10 08:21:01 +01:00
committed by GitHub
parent 33af6b8131 b00938e9b3
commit 18fd386bb4
4 changed files with 48 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
javascript/ql/src/semmle/javascript/frameworks/HTTP.qll
View File

@@ -125,6 +125,13 @@ module HTTP {
}
}
/**
* Gets the string `http` or `https`.
*/
string httpOrHttps() {
result = "http" or result = "https"
}
/**
* An expression whose value is sent as (part of) the body of an HTTP response.
*/

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

@@ -16,14 +16,19 @@ module NodeJSLib {
result = DataFlow::moduleImport("process")
}
/**
* Gets a reference to a member of the 'process' object.
*/
private DataFlow::SourceNode processMember(string member) {
result = process().getAPropertyRead(member) or
result = DataFlow::moduleMember("process", member)
}
/**
* Holds if `call` is an invocation of `http.createServer` or `https.createServer`.
*/
predicate isCreateServer(CallExpr call) {
exists (string name |
name = "http" or name = "https" |
call = DataFlow::moduleMember(name, "createServer").getAnInvocation().asExpr()
)
call = DataFlow::moduleMember(HTTP::httpOrHttps(), "createServer").getAnInvocation().asExpr()
}
/**
@@ -234,30 +239,27 @@ module NodeJSLib {
/**
* A call to a path-module method that preserves taint.
*/
private class PathFlowTarget extends TaintTracking::AdditionalTaintStep, DataFlow::ValueNode {
override CallExpr astNode;
Expr tainted;
private class PathFlowTarget extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
DataFlow::Node tainted;
PathFlowTarget() {
exists (DataFlow::ModuleImportNode pathModule, string methodName |
pathModule.getPath() = "path" and
this = pathModule.getAMemberCall(methodName) |
exists (string methodName | this = DataFlow::moduleMember("path", methodName).getACall() |
// getters
(methodName = "basename" and tainted = astNode.getArgument(0)) or
(methodName = "dirname" and tainted = astNode.getArgument(0)) or
(methodName = "extname" and tainted = astNode.getArgument(0)) or
(methodName = "basename" and tainted = getArgument(0)) or
(methodName = "dirname" and tainted = getArgument(0)) or
(methodName = "extname" and tainted = getArgument(0)) or
// transformers
(methodName = "join" and tainted = astNode.getAnArgument()) or
(methodName = "normalize" and tainted = astNode.getArgument(0)) or
(methodName = "relative" and tainted = astNode.getArgument([0..1])) or
(methodName = "resolve" and tainted = astNode.getAnArgument()) or
(methodName = "toNamespacedPath" and tainted = astNode.getArgument(0))
(methodName = "join" and tainted = getAnArgument()) or
(methodName = "normalize" and tainted = getArgument(0)) or
(methodName = "relative" and tainted = getArgument([0..1])) or
(methodName = "resolve" and tainted = getAnArgument()) or
(methodName = "toNamespacedPath" and tainted = getArgument(0))
)
}
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
pred.asExpr() = tainted and succ = this
pred = tainted and succ = this
}
}
@@ -294,13 +296,7 @@ module NodeJSLib {
class Credentials extends CredentialsExpr {
Credentials() {
exists (CallExpr call |
exists (DataFlow::ModuleImportNode http |
http.getPath() = "http" or http.getPath() = "https" |
call = http.getAMemberCall("request").asExpr()
) and
call.hasOptionArgument(0, "auth", this)
)
this = DataFlow::moduleMember(HTTP::httpOrHttps(), "request").getACall().getOptionArgument(0, "auth").asExpr()
}
override string getCredentialsKind() {
@@ -318,7 +314,7 @@ module NodeJSLib {
ProcessTermination() {
this = DataFlow::moduleImport("exit").getAnInvocation()
or
this = process().getAMemberCall("exit")
this = processMember("exit").getACall()
}
}
@@ -344,19 +340,18 @@ module NodeJSLib {
/**
* A call to a method from module `fs` or `graceful-fs`.
*/
private class NodeJSFileSystemAccess extends FileSystemAccess, DataFlow::ValueNode {
override MethodCallExpr astNode;
private class NodeJSFileSystemAccess extends FileSystemAccess, DataFlow::CallNode {
string methodName;
NodeJSFileSystemAccess() {
exists (DataFlow::ModuleImportNode fs |
fs.getPath() = "fs" or fs.getPath() = "graceful-fs" |
this = fs.getAMemberCall(_)
exists (string moduleName | this = DataFlow::moduleMember(moduleName, methodName).getACall() |
moduleName = "fs" or moduleName = "graceful-fs"
)
}
override DataFlow::Node getAPathArgument() {
exists (int i | fsFileParam(astNode.getMethodName(), i) |
result = DataFlow::valueNode(astNode.getArgument(i))
exists (int i | fsFileParam(methodName, i) |
result = getArgument(i)
)
}
}

11
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath-es6.js Normal file
View File

@@ -0,0 +1,11 @@
import { readFileSync } from 'fs';
import { createServer } from 'http';
import { parse } from 'url';
import { join } from 'path';
var server = createServer(function(req, res) {
let path = parse(req.url, true).query.path;
// BAD: This could read any file on the file system
res.write(readFileSync(join("public", path)));
});

1
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath.expected
View File

@@ -1,3 +1,4 @@
| TaintedPath-es6.js:10:26:10:45 | join("public", path) | This path depends on $@. | TaintedPath-es6.js:7:20:7:26 | req.url | a user-provided value |
| TaintedPath.js:12:29:12:32 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
| TaintedPath.js:15:29:15:48 | "/home/user/" + path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
| TaintedPath.js:19:33:19:36 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |