Skip to content

Commit a6d644b

Browse files
erik-krogherik-krogh
committed
add support for path.normalize(path.realtive(...))
1 parent 94814fa commit a6d644b

File tree

3 files changed

+56
-1
lines changed

3 files changed

+56
-1
lines changed

‎javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,16 @@ module TaintedPath {
367367
RelativePathContainsDotDotGuard() {
368368
this = startsWith and
369369
relativeCall = DataFlow::moduleImport("path").getAMemberCall("relative") and
370-
startsWith.getBaseString().getALocalSource() = relativeCall and
370+
(
371+
startsWith.getBaseString().getALocalSource() = relativeCall
372+
or
373+
startsWith
374+
.getBaseString()
375+
.getALocalSource()
376+
.(NormalizingPathCall)
377+
.getInput()
378+
.getALocalSource() = relativeCall
379+
) and
371380
exists(DataFlow::Node subString, string prefix |
372381
subString = startsWith.getSubstring() and
373382
(prefix = ".." or prefix = "../")

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,6 +1316,23 @@ nodes
13161316
| normalizedPaths.js:278:21:278:27 | newpath |
13171317
| normalizedPaths.js:278:21:278:27 | newpath |
13181318
| normalizedPaths.js:278:21:278:27 | newpath |
1319+
| normalizedPaths.js:283:7:283:42 | newpath |
1320+
| normalizedPaths.js:283:7:283:42 | newpath |
1321+
| normalizedPaths.js:283:7:283:42 | newpath |
1322+
| normalizedPaths.js:283:7:283:42 | newpath |
1323+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
1324+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
1325+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
1326+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
1327+
| normalizedPaths.js:283:38:283:41 | path |
1328+
| normalizedPaths.js:283:38:283:41 | path |
1329+
| normalizedPaths.js:283:38:283:41 | path |
1330+
| normalizedPaths.js:283:38:283:41 | path |
1331+
| normalizedPaths.js:286:21:286:27 | newpath |
1332+
| normalizedPaths.js:286:21:286:27 | newpath |
1333+
| normalizedPaths.js:286:21:286:27 | newpath |
1334+
| normalizedPaths.js:286:21:286:27 | newpath |
1335+
| normalizedPaths.js:286:21:286:27 | newpath |
13191336
| tainted-require.js:7:19:7:37 | req.param("module") |
13201337
| tainted-require.js:7:19:7:37 | req.param("module") |
13211338
| tainted-require.js:7:19:7:37 | req.param("module") |
@@ -3711,6 +3728,10 @@ edges
37113728
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:275:38:275:41 | path |
37123729
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:275:38:275:41 | path |
37133730
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:275:38:275:41 | path |
3731+
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:283:38:283:41 | path |
3732+
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:283:38:283:41 | path |
3733+
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:283:38:283:41 | path |
3734+
| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:283:38:283:41 | path |
37143735
| normalizedPaths.js:254:14:254:47 | pathMod ... y.path) | normalizedPaths.js:254:7:254:47 | path |
37153736
| normalizedPaths.js:254:14:254:47 | pathMod ... y.path) | normalizedPaths.js:254:7:254:47 | path |
37163737
| normalizedPaths.js:254:14:254:47 | pathMod ... y.path) | normalizedPaths.js:254:7:254:47 | path |
@@ -3755,6 +3776,22 @@ edges
37553776
| normalizedPaths.js:275:38:275:41 | path | normalizedPaths.js:275:17:275:42 | pathMod ... e(path) |
37563777
| normalizedPaths.js:275:38:275:41 | path | normalizedPaths.js:275:17:275:42 | pathMod ... e(path) |
37573778
| normalizedPaths.js:275:38:275:41 | path | normalizedPaths.js:275:17:275:42 | pathMod ... e(path) |
3779+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3780+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3781+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3782+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3783+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3784+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3785+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3786+
| normalizedPaths.js:283:7:283:42 | newpath | normalizedPaths.js:286:21:286:27 | newpath |
3787+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) | normalizedPaths.js:283:7:283:42 | newpath |
3788+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) | normalizedPaths.js:283:7:283:42 | newpath |
3789+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) | normalizedPaths.js:283:7:283:42 | newpath |
3790+
| normalizedPaths.js:283:17:283:42 | pathMod ... e(path) | normalizedPaths.js:283:7:283:42 | newpath |
3791+
| normalizedPaths.js:283:38:283:41 | path | normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
3792+
| normalizedPaths.js:283:38:283:41 | path | normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
3793+
| normalizedPaths.js:283:38:283:41 | path | normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
3794+
| normalizedPaths.js:283:38:283:41 | path | normalizedPaths.js:283:17:283:42 | pathMod ... e(path) |
37583795
| tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") |
37593796
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
37603797
| tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") |
@@ -4540,6 +4577,7 @@ edges
45404577
| normalizedPaths.js:262:21:262:24 | path | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:262:21:262:24 | path | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
45414578
| normalizedPaths.js:270:21:270:27 | newpath | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:270:21:270:27 | newpath | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
45424579
| normalizedPaths.js:278:21:278:27 | newpath | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:278:21:278:27 | newpath | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
4580+
| normalizedPaths.js:286:21:286:27 | newpath | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:286:21:286:27 | newpath | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
45434581
| 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 |
45444582
| 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 |
45454583
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,4 +279,12 @@ app.get('/relative-startswith', (req, res) => {
279279
} else {
280280
fs.readFileSync(newpath); // OK!
281281
}
282+
283+
let newpath = pathModule.normalize(path);
284+
var relativePath = pathModule.relative(pathModule.normalize(workspaceDir), newpath);
285+
if (pathModule.normalize(relativePath).indexOf('../') === 0) {
286+
fs.readFileSync(newpath); // NOT OK!
287+
} else {
288+
fs.readFileSync(newpath); // OK!
289+
}
282290
});

0 commit comments

Comments
 (0)