hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 08:45:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Consistently use the shared XSS barrier guards in the XSS queries

Browse Source 
Previously only reflected XSS used shared barrier guards.
This commit is contained in:
Asger F
2024-10-02 14:36:53 +02:00
parent 341bacfe55
commit 6cbe04dcb7
8 changed files with 13 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll
View File

@@ -55,7 +55,9 @@ module DomBasedXssConfig implements DataFlow::StateConfigSig {
label = prefixLabel()
}
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
predicate isBarrier(DataFlow::Node node) {
node instanceof Sanitizer or node = Shared::BarrierGuard::getABarrierNode()
}
predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
// copy all taint barrier guards to the TaintedUrlSuffix/PrefixLabel label

4
javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll
View File

@@ -140,7 +140,9 @@ module ExceptionXssConfig implements DataFlow::StateConfigSig {
sink instanceof XssShared::Sink and not label instanceof NotYetThrown
}
predicate isBarrier(DataFlow::Node node) { node instanceof XssShared::Sanitizer }
predicate isBarrier(DataFlow::Node node) {
node instanceof XssShared::Sanitizer or node = XssShared::BarrierGuard::getABarrierNode()
}
predicate isAdditionalFlowStep(
DataFlow::Node pred, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel outlbl

4
javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssQuery.qll
View File

@@ -15,7 +15,9 @@ module StoredXssConfig implements DataFlow::ConfigSig {
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
predicate isBarrier(DataFlow::Node node) {
node instanceof Sanitizer or node = Shared::BarrierGuard::getABarrierNode()
}
}
/**

2
javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll
View File

@@ -34,6 +34,8 @@ module UnsafeHtmlConstructionConfig implements DataFlow::StateConfigSig {
node instanceof UnsafeJQueryPlugin::Sanitizer
or
DomBasedXss::isOptionallySanitizedNode(node)
or
node = Shared::BarrierGuard::getABarrierNode()
}
predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) {

3
javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll
View File

@@ -22,7 +22,8 @@ module XssThroughDomConfig implements DataFlow::ConfigSig {
node instanceof DomBasedXss::Sanitizer or
DomBasedXss::isOptionallySanitizedNode(node) or
node = DataFlow::MakeBarrierGuard<BarrierGuard>::getABarrierNode() or
node = DataFlow::MakeBarrierGuard<UnsafeJQuery::BarrierGuard>::getABarrierNode()
node = DataFlow::MakeBarrierGuard<UnsafeJQuery::BarrierGuard>::getABarrierNode() or
node = Shared::BarrierGuard::getABarrierNode()
}
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {

3
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/ConsistencyDomBasedXss.expected
View File

@@ -1,3 +0,0 @@
| query-tests/Security/CWE-079/DomBasedXss/sanitiser.js:25 | did not expect an alert, but found an alert for HtmlInjection | OK | ConsistencyConfig |
| query-tests/Security/CWE-079/DomBasedXss/sanitiser.js:28 | did not expect an alert, but found an alert for HtmlInjection | OK | ConsistencyConfig |
| query-tests/Security/CWE-079/DomBasedXss/sanitiser.js:35 | did not expect an alert, but found an alert for HtmlInjection | OK | ConsistencyConfig |

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

@@ -284,16 +284,10 @@ nodes
| sanitiser.js:16:17:16:27 | window.name | semmle.label | window.name |
| sanitiser.js:23:21:23:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:23:29:23:35 | tainted | semmle.label | tainted |
| sanitiser.js:25:21:25:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:25:29:25:35 | tainted | semmle.label | tainted |
| sanitiser.js:28:21:28:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:28:29:28:35 | tainted | semmle.label | tainted |
| sanitiser.js:30:21:30:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:30:29:30:35 | tainted | semmle.label | tainted |
| sanitiser.js:33:21:33:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:33:29:33:35 | tainted | semmle.label | tainted |
| sanitiser.js:35:21:35:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:35:29:35:35 | tainted | semmle.label | tainted |
| sanitiser.js:38:21:38:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:38:29:38:35 | tainted | semmle.label | tainted |
| sanitiser.js:45:21:45:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
@@ -852,21 +846,15 @@ edges
| react-use-state.js:22:14:22:17 | prev | react-use-state.js:23:35:23:38 | prev | provenance | |
| react-use-state.js:25:20:25:30 | window.name | react-use-state.js:21:10:21:14 | state | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:23:29:23:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:25:29:25:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:28:29:28:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:30:29:30:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:33:29:33:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:35:29:35:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:38:29:38:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:45:29:45:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:48:19:48:25 | tainted | provenance | |
| sanitiser.js:16:17:16:27 | window.name | sanitiser.js:16:7:16:27 | tainted | provenance | |
| sanitiser.js:23:29:23:35 | tainted | sanitiser.js:23:21:23:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:25:29:25:35 | tainted | sanitiser.js:25:21:25:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:28:29:28:35 | tainted | sanitiser.js:28:21:28:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:30:29:30:35 | tainted | sanitiser.js:30:21:30:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:33:29:33:35 | tainted | sanitiser.js:33:21:33:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:35:29:35:35 | tainted | sanitiser.js:35:21:35:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:38:29:38:35 | tainted | sanitiser.js:38:21:38:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:45:29:45:35 | tainted | sanitiser.js:45:21:45:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:48:19:48:25 | tainted | sanitiser.js:48:19:48:46 | tainted ... /g, '') | provenance | |
@@ -1265,11 +1253,8 @@ subpaths
| react-use-state.js:17:51:17:55 | state | react-use-state.js:16:20:16:30 | window.name | react-use-state.js:17:51:17:55 | state | Cross-site scripting vulnerability due to $@. | react-use-state.js:16:20:16:30 | window.name | user-provided value |
| react-use-state.js:23:35:23:38 | prev | react-use-state.js:25:20:25:30 | window.name | react-use-state.js:23:35:23:38 | prev | Cross-site scripting vulnerability due to $@. | react-use-state.js:25:20:25:30 | window.name | user-provided value |
| sanitiser.js:23:21:23:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:23:21:23:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:25:21:25:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:25:21:25:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:28:21:28:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:28:21:28:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:30:21:30:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:30:21:30:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:33:21:33:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:33:21:33:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:35:21:35:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:35:21:35:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:38:21:38:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:38:21:38:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:45:21:45:44 | '<b>' + ... '</b>' | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:45:21:45:44 | '<b>' + ... '</b>' | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |
| sanitiser.js:48:19:48:46 | tainted ... /g, '') | sanitiser.js:16:17:16:27 | window.name | sanitiser.js:48:19:48:46 | tainted ... /g, '') | Cross-site scripting vulnerability due to $@. | sanitiser.js:16:17:16:27 | window.name | user-provided value |

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

@@ -289,16 +289,10 @@ nodes
| sanitiser.js:16:17:16:27 | window.name | semmle.label | window.name |
| sanitiser.js:23:21:23:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:23:29:23:35 | tainted | semmle.label | tainted |
| sanitiser.js:25:21:25:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:25:29:25:35 | tainted | semmle.label | tainted |
| sanitiser.js:28:21:28:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:28:29:28:35 | tainted | semmle.label | tainted |
| sanitiser.js:30:21:30:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:30:29:30:35 | tainted | semmle.label | tainted |
| sanitiser.js:33:21:33:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:33:29:33:35 | tainted | semmle.label | tainted |
| sanitiser.js:35:21:35:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:35:29:35:35 | tainted | semmle.label | tainted |
| sanitiser.js:38:21:38:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
| sanitiser.js:38:29:38:35 | tainted | semmle.label | tainted |
| sanitiser.js:45:21:45:44 | '<b>' + ... '</b>' | semmle.label | '<b>' + ... '</b>' |
@@ -876,21 +870,15 @@ edges
| react-use-state.js:22:14:22:17 | prev | react-use-state.js:23:35:23:38 | prev | provenance | |
| react-use-state.js:25:20:25:30 | window.name | react-use-state.js:21:10:21:14 | state | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:23:29:23:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:25:29:25:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:28:29:28:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:30:29:30:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:33:29:33:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:35:29:35:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:38:29:38:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:45:29:45:35 | tainted | provenance | |
| sanitiser.js:16:7:16:27 | tainted | sanitiser.js:48:19:48:25 | tainted | provenance | |
| sanitiser.js:16:17:16:27 | window.name | sanitiser.js:16:7:16:27 | tainted | provenance | |
| sanitiser.js:23:29:23:35 | tainted | sanitiser.js:23:21:23:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:25:29:25:35 | tainted | sanitiser.js:25:21:25:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:28:29:28:35 | tainted | sanitiser.js:28:21:28:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:30:29:30:35 | tainted | sanitiser.js:30:21:30:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:33:29:33:35 | tainted | sanitiser.js:33:21:33:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:35:29:35:35 | tainted | sanitiser.js:35:21:35:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:38:29:38:35 | tainted | sanitiser.js:38:21:38:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:45:29:45:35 | tainted | sanitiser.js:45:21:45:44 | '<b>' + ... '</b>' | provenance | |
| sanitiser.js:48:19:48:25 | tainted | sanitiser.js:48:19:48:46 | tainted ... /g, '') | provenance | |