From ecc008d175c15ad2a66f64e13e178d0fb6fd4df6 Mon Sep 17 00:00:00 2001 From: Napalys Date: Tue, 25 Feb 2025 12:59:22 +0100 Subject: [PATCH] Removed `appendChild` as potential sink as it does not accept raw HTML. --- javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll | 2 -- .../query-tests/Security/CWE-079/DomBasedXss/Xss.expected | 4 ---- .../Security/CWE-079/DomBasedXss/react-use-context.js | 4 ++-- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll index f959de6c0b5e..c290f164c990 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll @@ -56,8 +56,6 @@ class DomMethodCallNode extends DataFlow::MethodCallNode { name = "insertBefore" and argPos = 0 or name = "createElement" and argPos = 0 - or - name = "appendChild" and argPos = 0 ) } diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected index eb46033824f4..faf31ece1d62 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected @@ -257,8 +257,6 @@ nodes | react-native.js:7:17:7:33 | req.param("code") | semmle.label | req.param("code") | | react-native.js:8:18:8:24 | tainted | semmle.label | tainted | | react-native.js:9:27:9:33 | tainted | semmle.label | tainted | -| react-use-context.js:10:22:10:32 | window.name | semmle.label | window.name | -| react-use-context.js:16:26:16:36 | window.name | semmle.label | window.name | | react-use-router.js:8:21:8:32 | router.query | semmle.label | router.query | | react-use-router.js:8:21:8:39 | router.query.foobar | semmle.label | router.query.foobar | | react-use-router.js:11:24:11:35 | router.query | semmle.label | router.query | @@ -1322,8 +1320,6 @@ subpaths | pages/[id].jsx:16:44:16:51 | params.q | pages/[id].jsx:26:10:26:22 | context.query | pages/[id].jsx:16:44:16:51 | params.q | Cross-site scripting vulnerability due to $@. | pages/[id].jsx:26:10:26:22 | context.query | user-provided value | | react-native.js:8:18:8:24 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:8:18:8:24 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value | | react-native.js:9:27:9:33 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:9:27:9:33 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value | -| react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name | react-use-context.js:10:22:10:32 | window.name | Cross-site scripting vulnerability due to $@. | react-use-context.js:10:22:10:32 | window.name | user-provided value | -| react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name | react-use-context.js:16:26:16:36 | window.name | Cross-site scripting vulnerability due to $@. | react-use-context.js:16:26:16:36 | window.name | user-provided value | | react-use-router.js:8:21:8:39 | router.query.foobar | react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:8:21:8:32 | router.query | user-provided value | | react-use-router.js:11:24:11:42 | router.query.foobar | react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:11:24:11:35 | router.query | user-provided value | | react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar | Cross-site scripting vulnerability due to $@. | react-use-router.js:23:43:23:54 | router.query | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-context.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-context.js index 6d7e20ec6eb8..8c14b2ec3bc8 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-context.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/react-use-context.js @@ -7,13 +7,13 @@ function useMyContext() { export function useDoc1() { let { root } = useMyContext(); - root.appendChild(window.name); // NOT OK + root.appendChild(window.name); // OK -- the function does not accept string arguments } class C extends Component { foo() { let { root } = this.context; - root.appendChild(window.name); // NOT OK + root.appendChild(window.name); // OK -- the function does not accept string arguments } }