hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-04 13:15:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2723 from esbena/js/support-path-is-inside

Browse Source 
Approved by asgerf
This commit is contained in:
semmle-qlci
2020-02-25 11:21:24 +00:00
committed by GitHub
parent b453cf8f60 5baba62154
commit 03b882381a
4 changed files with 183 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
View File

@@ -35,7 +35,8 @@ module TaintedPath {
guard instanceof StartsWithDotDotSanitizer or
guard instanceof StartsWithDirSanitizer or
guard instanceof IsAbsoluteSanitizer or
guard instanceof ContainsDotDotSanitizer
guard instanceof ContainsDotDotSanitizer or
guard instanceof IsInsideCheckSanitizer
}
override predicate isAdditionalFlowStep(

33
javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
View File

@@ -369,6 +369,39 @@ module TaintedPath {
*/
private class VarAccessBarrier extends Sanitizer, DataFlow::VarAccessBarrier { }
/**
* An expression of form `isInside(x, y)` or similar, where `isInside` is
* a library check for the relation between `x` and `y`.
*/
class IsInsideCheckSanitizer extends DataFlow::LabeledBarrierGuardNode {
DataFlow::Node checked;
boolean onlyNormalizedAbsolutePaths;
IsInsideCheckSanitizer() {
exists(string name, DataFlow::CallNode check |
name = "path-is-inside" and onlyNormalizedAbsolutePaths = true
or
name = "is-path-inside" and onlyNormalizedAbsolutePaths = false
|
check = DataFlow::moduleImport(name).getACall() and
checked = check.getArgument(0) and
check = this
)
}
override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
(
onlyNormalizedAbsolutePaths = true and
label.(Label::PosixPath).isNormalized() and
label.(Label::PosixPath).isAbsolute()
or
onlyNormalizedAbsolutePaths = false
) and
e = checked.asExpr() and
outcome = true
}
}
/**
* A source of remote user input, considered as a flow source for
* tainted-path vulnerabilities.

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

@@ -1495,6 +1495,51 @@ nodes
| normalizedPaths.js:250:21:250:24 | path |
| normalizedPaths.js:250:21:250:24 | path |
| normalizedPaths.js:250:21:250:24 | path |
| normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path |
| normalizedPaths.js:256:13:256:26 | req.query.path |
| normalizedPaths.js:256:13:256:26 | req.query.path |
| normalizedPaths.js:256:13:256:26 | req.query.path |
| normalizedPaths.js:256:13:256:26 | req.query.path |
| normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:273:6:273:49 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath |
| normalizedPaths.js:273:23:273:49 | pathMod ... , path) |
| normalizedPaths.js:273:23:273:49 | pathMod ... , path) |
| normalizedPaths.js:273:23:273:49 | pathMod ... , path) |
| normalizedPaths.js:273:45:273:48 | path |
| normalizedPaths.js:273:45:273:48 | path |
| normalizedPaths.js:273:45:273:48 | path |
| normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:285:19:285:32 | normalizedPath |
| tainted-require.js:7:19:7:37 | req.param("module") |
| tainted-require.js:7:19:7:37 | req.param("module") |
| tainted-require.js:7:19:7:37 | req.param("module") |
@@ -4228,6 +4273,65 @@ edges
| normalizedPaths.js:236:33:236:46 | req.query.path | normalizedPaths.js:236:14:236:47 | pathMod ... y.path) |
| normalizedPaths.js:236:33:236:46 | req.query.path | normalizedPaths.js:236:14:236:47 | pathMod ... y.path) |
| normalizedPaths.js:236:33:236:46 | req.query.path | normalizedPaths.js:236:14:236:47 | pathMod ... y.path) |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:257:18:257:21 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:262:19:262:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:266:19:266:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:269:19:269:22 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:273:45:273:48 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:273:45:273:48 | path |
| normalizedPaths.js:256:6:256:26 | path | normalizedPaths.js:273:45:273:48 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:256:6:256:26 | path |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:278:19:278:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:273:6:273:49 | normalizedPath | normalizedPaths.js:285:19:285:32 | normalizedPath |
| normalizedPaths.js:273:23:273:49 | pathMod ... , path) | normalizedPaths.js:273:6:273:49 | normalizedPath |
| normalizedPaths.js:273:23:273:49 | pathMod ... , path) | normalizedPaths.js:273:6:273:49 | normalizedPath |
| normalizedPaths.js:273:23:273:49 | pathMod ... , path) | normalizedPaths.js:273:6:273:49 | normalizedPath |
| normalizedPaths.js:273:45:273:48 | path | normalizedPaths.js:273:23:273:49 | pathMod ... , path) |
| normalizedPaths.js:273:45:273:48 | path | normalizedPaths.js:273:23:273:49 | pathMod ... , path) |
| normalizedPaths.js:273:45:273:48 | path | normalizedPaths.js:273:23:273:49 | pathMod ... , path) |
| tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") |
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
| tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") |
@@ -5096,6 +5200,12 @@ edges
| normalizedPaths.js:238:19:238:22 | path | normalizedPaths.js:236:33:236:46 | req.query.path | normalizedPaths.js:238:19:238:22 | path | This path depends on $@. | normalizedPaths.js:236:33:236:46 | req.query.path | a user-provided value |
| normalizedPaths.js:245:21:245:24 | path | normalizedPaths.js:236:33:236:46 | req.query.path | normalizedPaths.js:245:21:245:24 | path | This path depends on $@. | normalizedPaths.js:236:33:236:46 | req.query.path | a user-provided value |
| normalizedPaths.js:250:21:250:24 | path | normalizedPaths.js:236:33:236:46 | req.query.path | normalizedPaths.js:250:21:250:24 | path | This path depends on $@. | normalizedPaths.js:236:33:236:46 | req.query.path | a user-provided value |
| normalizedPaths.js:257:18:257:21 | path | normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:257:18:257:21 | path | This path depends on $@. | normalizedPaths.js:256:13:256:26 | req.query.path | a user-provided value |
| normalizedPaths.js:262:19:262:22 | path | normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:262:19:262:22 | path | This path depends on $@. | normalizedPaths.js:256:13:256:26 | req.query.path | a user-provided value |
| normalizedPaths.js:266:19:266:22 | path | normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:266:19:266:22 | path | This path depends on $@. | normalizedPaths.js:256:13:256:26 | req.query.path | a user-provided value |
| normalizedPaths.js:269:19:269:22 | path | normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:269:19:269:22 | path | This path depends on $@. | normalizedPaths.js:256:13:256:26 | req.query.path | a user-provided value |
| normalizedPaths.js:278:19:278:32 | normalizedPath | normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:278:19:278:32 | normalizedPath | This path depends on $@. | normalizedPaths.js:256:13:256:26 | req.query.path | a user-provided value |
| normalizedPaths.js:285:19:285:32 | normalizedPath | normalizedPaths.js:256:13:256:26 | req.query.path | normalizedPaths.js:285:19:285:32 | normalizedPath | This path depends on $@. | normalizedPaths.js:256:13:256:26 | req.query.path | a user-provided value |
| tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") | This path depends on $@. | tainted-require.js:7:19:7:37 | req.param("module") | a user-provided value |
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | This path depends on $@. | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | a user-provided value |
| tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | This path depends on $@. | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | a user-provided value |

38
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js
View File

@@ -249,3 +249,41 @@ app.get('/resolve-path', (req, res) => {
else
fs.readFileSync(path); // NOT OK - wrong polarity
});
var isPathInside = require("is-path-inside"),
pathIsInside = require("path-is-inside");
app.get('/pseudo-normalizations', (req, res) => {
let path = req.query.path;
fs.readFileSync(path); // NOT OK
if (isPathInside(path, SAFE)) {
fs.readFileSync(path); // OK
return;
} else {
fs.readFileSync(path); // NOT OK
}
if (pathIsInside(path, SAFE)) {
fs.readFileSync(path); // NOT OK - can be of the form 'safe/directory/../../../etc/passwd'
return;
} else {
fs.readFileSync(path); // NOT OK
}
let normalizedPath = pathModule.join(SAFE, path);
if (pathIsInside(normalizedPath, SAFE)) {
fs.readFileSync(normalizedPath); // OK
return;
} else {
fs.readFileSync(normalizedPath); // NOT OK
}
if (pathIsInside(normalizedPath, SAFE)) {
fs.readFileSync(normalizedPath); // OK
return;
} else {
fs.readFileSync(normalizedPath); // NOT OK
}
});