diff --git a/javascript/ql/lib/semmle/javascript/DOM.qll b/javascript/ql/lib/semmle/javascript/DOM.qll index 50a529b4a53a..21854c1a9d0e 100644 --- a/javascript/ql/lib/semmle/javascript/DOM.qll +++ b/javascript/ql/lib/semmle/javascript/DOM.qll @@ -247,7 +247,7 @@ module DOM { ] | ( - result = documentRef().getAMethodCall(collectionName) or + result = domValueRef().getAMethodCall(collectionName) or result = DataFlow::globalVarRef(collectionName).getACall() ) ) @@ -441,10 +441,12 @@ module DOM { DataFlow::SourceNode domValueRef() { result = domValueRef(DataFlow::TypeTracker::end()) or - result.hasUnderlyingType("Element") + result.hasUnderlyingType(["Element", "HTMLCollection", "HTMLCollectionOf"]) or result.hasUnderlyingType(any(string s | s.matches("HTML%Element"))) or + result = documentRef() + or exists(DataFlow::ClassNode cls | cls.getASuperClassNode().getALocalSource() = DataFlow::globalVarRef(any(string s | s.matches("HTML%Element"))) and diff --git a/javascript/ql/src/Security/CWE-327/BadRandomness.ql b/javascript/ql/src/Security/CWE-327/BadRandomness.ql index dd6de6cc1c01..5de0f9b3dfd9 100644 --- a/javascript/ql/src/Security/CWE-327/BadRandomness.ql +++ b/javascript/ql/src/Security/CWE-327/BadRandomness.ql @@ -30,30 +30,26 @@ private int powerOfTwo() { * Gets a node that has value 2^n for some n. */ private DataFlow::Node isPowerOfTwo() { - exists(DataFlow::Node prev | - prev.getIntValue() = powerOfTwo() - or - // Getting around the 32 bit ints in QL. These are some hex values of the form 0x10000000 - prev.asExpr().(NumberLiteral).getValue() = - ["281474976710656", "17592186044416", "1099511627776", "68719476736", "4294967296"] - | - result = prev.getASuccessor*() - ) + result.getIntValue() = powerOfTwo() + or + // Getting around the 32 bit ints in QL. These are some hex values of the form 0x10000000 + result.asExpr().(NumberLiteral).getValue() = + ["281474976710656", "17592186044416", "1099511627776", "68719476736", "4294967296"] + or + result = isPowerOfTwo().getASuccessor() } /** * Gets a node that has value (2^n)-1 for some n. */ private DataFlow::Node isPowerOfTwoMinusOne() { - exists(DataFlow::Node prev | - prev.getIntValue() = powerOfTwo() - 1 - or - // Getting around the 32 bit ints in QL. These are some hex values of the form 0xfffffff - prev.asExpr().(NumberLiteral).getValue() = - ["281474976710655", "17592186044415", "1099511627775", "68719476735", "4294967295"] - | - result = prev.getASuccessor*() - ) + result.getIntValue() = powerOfTwo() - 1 + or + // Getting around the 32 bit ints in QL. These are some hex values of the form 0xfffffff + result.asExpr().(NumberLiteral).getValue() = + ["281474976710655", "17592186044415", "1099511627775", "68719476735", "4294967295"] + or + result = isPowerOfTwoMinusOne().getASuccessor() } /** diff --git a/javascript/ql/src/change-notes/2025-04-02-name-resolution-independent-fixes.md b/javascript/ql/src/change-notes/2025-04-02-name-resolution-independent-fixes.md new file mode 100644 index 000000000000..4773744a984f --- /dev/null +++ b/javascript/ql/src/change-notes/2025-04-02-name-resolution-independent-fixes.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Slightly improved detection of DOM element references, leading to XSS results being detected in more cases. diff --git a/javascript/ql/test/library-tests/DOM/Customizations.expected b/javascript/ql/test/library-tests/DOM/Customizations.expected index 3fc5570c743c..df15c409be2d 100644 --- a/javascript/ql/test/library-tests/DOM/Customizations.expected +++ b/javascript/ql/test/library-tests/DOM/Customizations.expected @@ -7,20 +7,26 @@ test_documentRef test_locationRef | customization.js:3:3:3:14 | doc.location | test_domValueRef +| customization.js:2:13:2:31 | customGetDocument() | +| customization.js:3:3:3:14 | doc.location | | customization.js:4:3:4:20 | doc.getElementById | | customization.js:4:3:4:28 | doc.get ... 'test') | | event-handler-receiver.html:4:20:4:19 | this | +| event-handler-receiver.js:1:1:1:8 | document | | event-handler-receiver.js:1:1:1:23 | documen ... entById | | event-handler-receiver.js:1:1:1:32 | documen ... my-id') | | event-handler-receiver.js:1:44:1:43 | this | | event-handler-receiver.js:2:3:2:17 | this.parentNode | +| event-handler-receiver.js:5:1:5:8 | document | | event-handler-receiver.js:5:1:5:23 | documen ... entById | | event-handler-receiver.js:5:1:5:32 | documen ... my-id') | | event-handler-receiver.js:5:60:5:59 | this | | event-handler-receiver.js:6:3:6:17 | this.parentNode | +| nameditems.js:1:1:1:8 | document | | nameditems.js:1:1:1:23 | documen ... entById | | nameditems.js:1:1:1:30 | documen ... ('foo') | | nameditems.js:1:1:2:19 | documen ... em('x') | +| querySelectorAll.js:2:5:2:12 | document | | querySelectorAll.js:2:5:2:29 | documen ... ctorAll | | querySelectorAll.js:2:5:2:36 | documen ... ('foo') | | querySelectorAll.js:2:46:2:48 | elm | 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 7de1561f79e8..6ba8ab703bff 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 @@ -53,6 +53,7 @@ | dates.js:57:31:57:101 | `Time i ... aint)}` | dates.js:54:36:54:55 | window.location.hash | dates.js:57:31:57:101 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:54:36:54:55 | window.location.hash | user-provided value | | dates.js:59:31:59:87 | `Time i ... aint)}` | dates.js:54:36:54:55 | window.location.hash | dates.js:59:31:59:87 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:54:36:54:55 | window.location.hash | user-provided value | | dates.js:61:31:61:88 | `Time i ... aint)}` | dates.js:54:36:54:55 | window.location.hash | dates.js:61:31:61:88 | `Time i ... aint)}` | Cross-site scripting vulnerability due to $@. | dates.js:54:36:54:55 | window.location.hash | user-provided value | +| dom.js:4:20:4:30 | window.name | dom.js:4:20:4:30 | window.name | dom.js:4:20:4:30 | window.name | Cross-site scripting vulnerability due to $@. | dom.js:4:20:4:30 | window.name | user-provided value | | dragAndDrop.ts:15:25:15:28 | html | dragAndDrop.ts:8:18:8:50 | dataTra ... /html') | dragAndDrop.ts:15:25:15:28 | html | Cross-site scripting vulnerability due to $@. | dragAndDrop.ts:8:18:8:50 | dataTra ... /html') | user-provided value | | dragAndDrop.ts:24:23:24:57 | e.dataT ... /html') | dragAndDrop.ts:24:23:24:57 | e.dataT ... /html') | dragAndDrop.ts:24:23:24:57 | e.dataT ... /html') | Cross-site scripting vulnerability due to $@. | dragAndDrop.ts:24:23:24:57 | e.dataT ... /html') | user-provided value | | dragAndDrop.ts:29:19:29:53 | e.dataT ... /html') | dragAndDrop.ts:29:19:29:53 | e.dataT ... /html') | dragAndDrop.ts:29:19:29:53 | e.dataT ... /html') | Cross-site scripting vulnerability due to $@. | dragAndDrop.ts:29:19:29:53 | e.dataT ... /html') | user-provided value | @@ -937,6 +938,7 @@ nodes | dates.js:61:31:61:88 | `Time i ... aint)}` | semmle.label | `Time i ... aint)}` | | dates.js:61:42:61:86 | dayjs.s ... (taint) | semmle.label | dayjs.s ... (taint) | | dates.js:61:81:61:85 | taint | semmle.label | taint | +| dom.js:4:20:4:30 | window.name | semmle.label | window.name | | dragAndDrop.ts:8:11:8:50 | html | semmle.label | html | | dragAndDrop.ts:8:18:8:50 | dataTra ... /html') | semmle.label | dataTra ... /html') | | dragAndDrop.ts:15:25:15:28 | html | semmle.label | html | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected index ed2611a11e37..0ed15b8d92ab 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected @@ -138,6 +138,7 @@ nodes | dates.js:61:31:61:88 | `Time i ... aint)}` | semmle.label | `Time i ... aint)}` | | dates.js:61:42:61:86 | dayjs.s ... (taint) | semmle.label | dayjs.s ... (taint) | | dates.js:61:81:61:85 | taint | semmle.label | taint | +| dom.js:4:20:4:30 | window.name | semmle.label | window.name | | dragAndDrop.ts:8:11:8:50 | html | semmle.label | html | | dragAndDrop.ts:8:18:8:50 | dataTra ... /html') | semmle.label | dataTra ... /html') | | dragAndDrop.ts:15:25:15:28 | html | semmle.label | html | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/dom.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/dom.js new file mode 100644 index 000000000000..cf4f0bb3fbe2 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/dom.js @@ -0,0 +1,5 @@ +function t1() { + const elm = document.getElementById("foo"); + const e2 = elm.getElementsByTagName("bar")[0]; + e2.innerHTML = window.name; // $ Alert +}