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

JS: Do not use legacy taint steps in TaintedUrlSuffix

Browse Source 
Tainted URL suffix steps are added as configuration-specific additional
steps, which means implicit reads may occur before any of these steps.

These steps accidentally included the legacy taint steps which include
a step from 'arguments' to all positional parameters. Combined with the
implicit read, arguments could escape their array index and flow to
any parameter while in the tainted-url flow state.
This commit is contained in:
Asger F
2024-08-29 13:36:44 +02:00
parent 65a36b0b3b
commit 4568967a76
4 changed files with 3 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll
View File

@@ -54,7 +54,7 @@ module TaintedUrlSuffix {
// Inherit all ordinary taint steps except `x -> x.p` steps
srclbl = label() and
dstlbl = label() and
TaintTracking::sharedTaintStep(src, dst) and
TaintTracking::AdditionalTaintStep::step(src, dst) and
not isSafeLocationProp(dst)
or
// Transition from URL suffix to full taint when extracting the query/fragment part.

18
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected
View File

@@ -330,13 +330,8 @@ nodes
| string-manipulations.js:9:36:9:57 | documen ... on.href | semmle.label | documen ... on.href |
| string-manipulations.js:10:16:10:45 | String( ... n.href) | semmle.label | String( ... n.href) |
| string-manipulations.js:10:23:10:44 | documen ... on.href | semmle.label | documen ... on.href |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | semmle.label | 'arguments' object of function foo [1] |
| tainted-url-suffix-arguments.js:3:14:3:14 | x | semmle.label | x |
| tainted-url-suffix-arguments.js:3:17:3:17 | y | semmle.label | y |
| tainted-url-suffix-arguments.js:3:20:3:20 | z | semmle.label | z |
| tainted-url-suffix-arguments.js:5:22:5:22 | x | semmle.label | x |
| tainted-url-suffix-arguments.js:6:22:6:22 | y | semmle.label | y |
| tainted-url-suffix-arguments.js:7:22:7:22 | z | semmle.label | z |
| tainted-url-suffix-arguments.js:11:11:11:36 | url | semmle.label | url |
| tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | semmle.label | window.location.href |
| tainted-url-suffix-arguments.js:12:17:12:19 | url | semmle.label | url |
@@ -426,7 +421,6 @@ nodes
| tst.js:64:25:64:48 | documen ... .search | semmle.label | documen ... .search |
| tst.js:65:25:65:48 | documen ... .search | semmle.label | documen ... .search |
| tst.js:68:16:68:20 | bar() | semmle.label | bar() |
| tst.js:70:1:70:27 | [,docum ... search] | semmle.label | [,docum ... search] |
| tst.js:70:1:70:27 | [,docum ... search] [1] | semmle.label | [,docum ... search] [1] |
| tst.js:70:3:70:26 | documen ... .search | semmle.label | documen ... .search |
| tst.js:70:46:70:46 | x | semmle.label | x |
@@ -959,15 +953,9 @@ edges
| string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | provenance | Config |
| string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | |
| string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | Config |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | tainted-url-suffix-arguments.js:3:14:3:14 | x | provenance | Config |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | tainted-url-suffix-arguments.js:3:17:3:17 | y | provenance | Config |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | tainted-url-suffix-arguments.js:3:20:3:20 | z | provenance | Config |
| tainted-url-suffix-arguments.js:3:14:3:14 | x | tainted-url-suffix-arguments.js:5:22:5:22 | x | provenance | |
| tainted-url-suffix-arguments.js:3:17:3:17 | y | tainted-url-suffix-arguments.js:6:22:6:22 | y | provenance | |
| tainted-url-suffix-arguments.js:3:20:3:20 | z | tainted-url-suffix-arguments.js:7:22:7:22 | z | provenance | |
| tainted-url-suffix-arguments.js:11:11:11:36 | url | tainted-url-suffix-arguments.js:12:17:12:19 | url | provenance | |
| tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:11:11:11:36 | url | provenance | |
| tainted-url-suffix-arguments.js:12:17:12:19 | url | tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | provenance | |
| tainted-url-suffix-arguments.js:12:17:12:19 | url | tainted-url-suffix-arguments.js:3:17:3:17 | y | provenance | |
| tooltip.jsx:6:11:6:30 | source | tooltip.jsx:10:25:10:30 | source | provenance | |
| tooltip.jsx:6:11:6:30 | source | tooltip.jsx:11:25:11:30 | source | provenance | |
@@ -1053,11 +1041,7 @@ edges
| tst.js:60:34:60:34 | s | tst.js:62:18:62:18 | s | provenance | |
| tst.js:64:25:64:48 | documen ... .search | tst.js:60:34:60:34 | s | provenance | |
| tst.js:65:25:65:48 | documen ... .search | tst.js:60:34:60:34 | s | provenance | |
| tst.js:70:1:70:27 | [,docum ... search] | tst.js:70:46:70:46 | x | provenance | |
| tst.js:70:1:70:27 | [,docum ... search] | tst.js:70:46:70:46 | x | provenance | Config |
| tst.js:70:1:70:27 | [,docum ... search] [1] | tst.js:70:46:70:46 | x | provenance | |
| tst.js:70:1:70:27 | [,docum ... search] [1] | tst.js:70:46:70:46 | x | provenance | Config |
| tst.js:70:3:70:26 | documen ... .search | tst.js:70:1:70:27 | [,docum ... search] | provenance | Config |
| tst.js:70:3:70:26 | documen ... .search | tst.js:70:1:70:27 | [,docum ... search] [1] | provenance | |
| tst.js:70:46:70:46 | x | tst.js:73:20:73:20 | x | provenance | |
| tst.js:107:7:107:44 | v | tst.js:110:18:110:18 | v | provenance | |
@@ -1398,9 +1382,7 @@ subpaths
| string-manipulations.js:8:16:8:48 | documen ... mLeft() | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | Cross-site scripting vulnerability due to $@. | string-manipulations.js:8:16:8:37 | documen ... on.href | user-provided value |
| string-manipulations.js:9:16:9:58 | String. ... n.href) | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:9:36:9:57 | documen ... on.href | user-provided value |
| string-manipulations.js:10:16:10:45 | String( ... n.href) | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:10:23:10:44 | documen ... on.href | user-provided value |
| tainted-url-suffix-arguments.js:5:22:5:22 | x | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:5:22:5:22 | x | Cross-site scripting vulnerability due to $@. | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | user-provided value |
| tainted-url-suffix-arguments.js:6:22:6:22 | y | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:6:22:6:22 | y | Cross-site scripting vulnerability due to $@. | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | user-provided value |
| tainted-url-suffix-arguments.js:7:22:7:22 | z | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:7:22:7:22 | z | Cross-site scripting vulnerability due to $@. | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | user-provided value |
| tooltip.jsx:10:25:10:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:10:25:10:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value |
| tooltip.jsx:11:25:11:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:11:25:11:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value |
| tooltip.jsx:18:51:18:59 | provide() | tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:18:51:18:59 | provide() | Cross-site scripting vulnerability due to $@. | tooltip.jsx:22:20:22:30 | window.name | user-provided value |

16
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected
View File

@@ -335,13 +335,8 @@ nodes
| string-manipulations.js:9:36:9:57 | documen ... on.href | semmle.label | documen ... on.href |
| string-manipulations.js:10:16:10:45 | String( ... n.href) | semmle.label | String( ... n.href) |
| string-manipulations.js:10:23:10:44 | documen ... on.href | semmle.label | documen ... on.href |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | semmle.label | 'arguments' object of function foo [1] |
| tainted-url-suffix-arguments.js:3:14:3:14 | x | semmle.label | x |
| tainted-url-suffix-arguments.js:3:17:3:17 | y | semmle.label | y |
| tainted-url-suffix-arguments.js:3:20:3:20 | z | semmle.label | z |
| tainted-url-suffix-arguments.js:5:22:5:22 | x | semmle.label | x |
| tainted-url-suffix-arguments.js:6:22:6:22 | y | semmle.label | y |
| tainted-url-suffix-arguments.js:7:22:7:22 | z | semmle.label | z |
| tainted-url-suffix-arguments.js:11:11:11:36 | url | semmle.label | url |
| tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | semmle.label | window.location.href |
| tainted-url-suffix-arguments.js:12:17:12:19 | url | semmle.label | url |
@@ -431,7 +426,6 @@ nodes
| tst.js:64:25:64:48 | documen ... .search | semmle.label | documen ... .search |
| tst.js:65:25:65:48 | documen ... .search | semmle.label | documen ... .search |
| tst.js:68:16:68:20 | bar() | semmle.label | bar() |
| tst.js:70:1:70:27 | [,docum ... search] | semmle.label | [,docum ... search] |
| tst.js:70:1:70:27 | [,docum ... search] [1] | semmle.label | [,docum ... search] [1] |
| tst.js:70:3:70:26 | documen ... .search | semmle.label | documen ... .search |
| tst.js:70:46:70:46 | x | semmle.label | x |
@@ -984,15 +978,9 @@ edges
| string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | provenance | Config |
| string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | |
| string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | Config |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | tainted-url-suffix-arguments.js:3:14:3:14 | x | provenance | Config |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | tainted-url-suffix-arguments.js:3:17:3:17 | y | provenance | Config |
| tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | tainted-url-suffix-arguments.js:3:20:3:20 | z | provenance | Config |
| tainted-url-suffix-arguments.js:3:14:3:14 | x | tainted-url-suffix-arguments.js:5:22:5:22 | x | provenance | |
| tainted-url-suffix-arguments.js:3:17:3:17 | y | tainted-url-suffix-arguments.js:6:22:6:22 | y | provenance | |
| tainted-url-suffix-arguments.js:3:20:3:20 | z | tainted-url-suffix-arguments.js:7:22:7:22 | z | provenance | |
| tainted-url-suffix-arguments.js:11:11:11:36 | url | tainted-url-suffix-arguments.js:12:17:12:19 | url | provenance | |
| tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:11:11:11:36 | url | provenance | |
| tainted-url-suffix-arguments.js:12:17:12:19 | url | tainted-url-suffix-arguments.js:3:1:8:1 | 'arguments' object of function foo [1] | provenance | |
| tainted-url-suffix-arguments.js:12:17:12:19 | url | tainted-url-suffix-arguments.js:3:17:3:17 | y | provenance | |
| tooltip.jsx:6:11:6:30 | source | tooltip.jsx:10:25:10:30 | source | provenance | |
| tooltip.jsx:6:11:6:30 | source | tooltip.jsx:11:25:11:30 | source | provenance | |
@@ -1078,11 +1066,7 @@ edges
| tst.js:60:34:60:34 | s | tst.js:62:18:62:18 | s | provenance | |
| tst.js:64:25:64:48 | documen ... .search | tst.js:60:34:60:34 | s | provenance | |
| tst.js:65:25:65:48 | documen ... .search | tst.js:60:34:60:34 | s | provenance | |
| tst.js:70:1:70:27 | [,docum ... search] | tst.js:70:46:70:46 | x | provenance | |
| tst.js:70:1:70:27 | [,docum ... search] | tst.js:70:46:70:46 | x | provenance | Config |
| tst.js:70:1:70:27 | [,docum ... search] [1] | tst.js:70:46:70:46 | x | provenance | |
| tst.js:70:1:70:27 | [,docum ... search] [1] | tst.js:70:46:70:46 | x | provenance | Config |
| tst.js:70:3:70:26 | documen ... .search | tst.js:70:1:70:27 | [,docum ... search] | provenance | Config |
| tst.js:70:3:70:26 | documen ... .search | tst.js:70:1:70:27 | [,docum ... search] [1] | provenance | |
| tst.js:70:46:70:46 | x | tst.js:73:20:73:20 | x | provenance | |
| tst.js:107:7:107:44 | v | tst.js:110:18:110:18 | v | provenance | |

4
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/tainted-url-suffix-arguments.js
View File

@@ -2,9 +2,9 @@ import 'dummy';
function foo(x, y, z) {
arguments; // ensure 'arguments' are used
document.writeln(x); // OK [INCONSISTENCY]
document.writeln(x); // OK
document.writeln(y); // NOT OK
document.writeln(z); // OK [INCONSISTENCY]
document.writeln(z); // OK
}
function bar() {