From e3ab5bdd1632cf606bfd01d388cf7124a3236742 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 5 Oct 2023 09:20:09 +0200 Subject: [PATCH] JS: Port IncompleteHtmlAttributeSanitization --- ...completeHtmlAttributeSanitizationQuery.qll | 29 +++++++- .../IncompleteHtmlAttributeSanitization.ql | 8 +- ...completeHtmlAttributeSanitization.expected | 73 +++++-------------- 3 files changed, 50 insertions(+), 60 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll index 730fa6a0e80..824d689445e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll @@ -25,7 +25,34 @@ private module Label { /** * A taint-tracking configuration for reasoning about incomplete HTML sanitization vulnerabilities. */ -class Configuration extends TaintTracking::Configuration { +module IncompleteHtmlAttributeSanitizationConfig implements DataFlow::StateConfigSig { + class FlowState = DataFlow::FlowLabel; + + predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { + label = Label::characterToLabel(source.(Source).getAnUnsanitizedCharacter()) + } + + predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { + label = Label::characterToLabel(sink.(Sink).getADangerousCharacter()) + } + + predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) { + lbl = Label::characterToLabel(node.(StringReplaceCall).getAReplacedString()) + } + + predicate isBarrier(DataFlow::Node n) { n instanceof Sanitizer } +} + +/** + * Taint-tracking for reasoning about incomplete HTML sanitization vulnerabilities. + */ +module IncompleteHtmlAttributeSanitizationFlow = + TaintTracking::GlobalWithState; + +/** + * DEPRECATED. Use the `IncompleteHtmlAttributeSanitizationFlow` module instead. + */ +deprecated class Configuration extends TaintTracking::Configuration { Configuration() { this = "IncompleteHtmlAttributeSanitization" } override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { diff --git a/javascript/ql/src/Security/CWE-116/IncompleteHtmlAttributeSanitization.ql b/javascript/ql/src/Security/CWE-116/IncompleteHtmlAttributeSanitization.ql index eec14ab7ba3..46b60ea9c99 100644 --- a/javascript/ql/src/Security/CWE-116/IncompleteHtmlAttributeSanitization.ql +++ b/javascript/ql/src/Security/CWE-116/IncompleteHtmlAttributeSanitization.ql @@ -15,9 +15,9 @@ */ import javascript -import DataFlow::PathGraph import semmle.javascript.security.dataflow.IncompleteHtmlAttributeSanitizationQuery import semmle.javascript.security.IncompleteBlacklistSanitizer +import DataFlow::DeduplicatePathGraph /** * Gets a pretty string of the dangerous characters for `sink`. @@ -31,8 +31,10 @@ string prettyPrintDangerousCharaters(Sink sink) { ).regexpReplaceAll(",(?=[^,]+$)", " or") } -from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) +from PathNode source, PathNode sink +where + IncompleteHtmlAttributeSanitizationFlow::flowPath(source.getAnOriginalPathNode(), + sink.getAnOriginalPathNode()) select sink.getNode(), source, sink, // this message is slightly sub-optimal as we do not have an easy way // to get the flow labels that reach the sink, so the message includes diff --git a/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteHtmlAttributeSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteHtmlAttributeSanitization.expected index 7c80b54be34..326e6ea7436 100644 --- a/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteHtmlAttributeSanitization.expected +++ b/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteHtmlAttributeSanitization.expected @@ -1,64 +1,25 @@ nodes -| tst.js:243:9:243:31 | s().rep ... ]/g,'') | -| tst.js:243:9:243:31 | s().rep ... ]/g,'') | -| tst.js:243:9:243:31 | s().rep ... ]/g,'') | -| tst.js:244:9:244:33 | s().rep ... /g, '') | -| tst.js:244:9:244:33 | s().rep ... /g, '') | -| tst.js:244:9:244:33 | s().rep ... /g, '') | -| tst.js:249:9:249:33 | s().rep ... ]/g,'') | -| tst.js:249:9:249:33 | s().rep ... ]/g,'') | -| tst.js:249:9:249:33 | s().rep ... ]/g,'') | -| tst.js:250:9:250:33 | s().rep ... ]/g,'') | -| tst.js:250:9:250:33 | s().rep ... ]/g,'') | -| tst.js:250:9:250:33 | s().rep ... ]/g,'') | -| tst.js:253:21:253:45 | s().rep ... /g, '') | -| tst.js:253:21:253:45 | s().rep ... /g, '') | -| tst.js:253:21:253:45 | s().rep ... /g, '') | -| tst.js:254:32:254:56 | s().rep ... /g, '') | -| tst.js:254:32:254:56 | s().rep ... /g, '') | -| tst.js:254:32:254:56 | s().rep ... /g, '') | -| tst.js:270:61:270:85 | s().rep ... /g, '') | -| tst.js:270:61:270:85 | s().rep ... /g, '') | -| tst.js:270:61:270:85 | s().rep ... /g, '') | -| tst.js:274:6:274:94 | arr | -| tst.js:274:12:274:94 | s().val ... g , '') | -| tst.js:274:12:274:94 | s().val ... g , '') | -| tst.js:275:9:275:11 | arr | -| tst.js:275:9:275:21 | arr.join(" ") | -| tst.js:275:9:275:21 | arr.join(" ") | -| tst.js:300:10:300:33 | s().rep ... ]/g,'') | -| tst.js:300:10:300:33 | s().rep ... ]/g,'') | -| tst.js:300:10:300:33 | s().rep ... ]/g,'') | -| tst.js:301:10:301:32 | s().rep ... ]/g,'') | -| tst.js:301:10:301:32 | s().rep ... ]/g,'') | -| tst.js:301:10:301:32 | s().rep ... ]/g,'') | -| tst.js:302:10:302:34 | s().rep ... ]/g,'') | -| tst.js:302:10:302:34 | s().rep ... ]/g,'') | -| tst.js:302:10:302:34 | s().rep ... ]/g,'') | -| tst.js:303:10:303:34 | s().rep ... /g, '') | -| tst.js:303:10:303:34 | s().rep ... /g, '') | -| tst.js:303:10:303:34 | s().rep ... /g, '') | -| tst.js:309:10:318:3 | s().rep ... ;";\\n\\t}) | -| tst.js:309:10:318:3 | s().rep ... ;";\\n\\t}) | -| tst.js:309:10:318:3 | s().rep ... ;";\\n\\t}) | +| tst.js:243:9:243:31 | s().rep ... ]/g,'') | semmle.label | s().rep ... ]/g,'') | +| tst.js:244:9:244:33 | s().rep ... /g, '') | semmle.label | s().rep ... /g, '') | +| tst.js:249:9:249:33 | s().rep ... ]/g,'') | semmle.label | s().rep ... ]/g,'') | +| tst.js:250:9:250:33 | s().rep ... ]/g,'') | semmle.label | s().rep ... ]/g,'') | +| tst.js:253:21:253:45 | s().rep ... /g, '') | semmle.label | s().rep ... /g, '') | +| tst.js:254:32:254:56 | s().rep ... /g, '') | semmle.label | s().rep ... /g, '') | +| tst.js:270:61:270:85 | s().rep ... /g, '') | semmle.label | s().rep ... /g, '') | +| tst.js:274:6:274:94 | arr | semmle.label | arr | +| tst.js:274:12:274:94 | s().val ... g , '') | semmle.label | s().val ... g , '') | +| tst.js:275:9:275:11 | arr | semmle.label | arr | +| tst.js:275:9:275:21 | arr.join(" ") | semmle.label | arr.join(" ") | +| tst.js:300:10:300:33 | s().rep ... ]/g,'') | semmle.label | s().rep ... ]/g,'') | +| tst.js:301:10:301:32 | s().rep ... ]/g,'') | semmle.label | s().rep ... ]/g,'') | +| tst.js:302:10:302:34 | s().rep ... ]/g,'') | semmle.label | s().rep ... ]/g,'') | +| tst.js:303:10:303:34 | s().rep ... /g, '') | semmle.label | s().rep ... /g, '') | +| tst.js:309:10:318:3 | s().rep ... ;";\\n\\t}) | semmle.label | s().rep ... ;";\\n\\t}) | edges -| tst.js:243:9:243:31 | s().rep ... ]/g,'') | tst.js:243:9:243:31 | s().rep ... ]/g,'') | -| tst.js:244:9:244:33 | s().rep ... /g, '') | tst.js:244:9:244:33 | s().rep ... /g, '') | -| tst.js:249:9:249:33 | s().rep ... ]/g,'') | tst.js:249:9:249:33 | s().rep ... ]/g,'') | -| tst.js:250:9:250:33 | s().rep ... ]/g,'') | tst.js:250:9:250:33 | s().rep ... ]/g,'') | -| tst.js:253:21:253:45 | s().rep ... /g, '') | tst.js:253:21:253:45 | s().rep ... /g, '') | -| tst.js:254:32:254:56 | s().rep ... /g, '') | tst.js:254:32:254:56 | s().rep ... /g, '') | -| tst.js:270:61:270:85 | s().rep ... /g, '') | tst.js:270:61:270:85 | s().rep ... /g, '') | | tst.js:274:6:274:94 | arr | tst.js:275:9:275:11 | arr | | tst.js:274:12:274:94 | s().val ... g , '') | tst.js:274:6:274:94 | arr | -| tst.js:274:12:274:94 | s().val ... g , '') | tst.js:274:6:274:94 | arr | | tst.js:275:9:275:11 | arr | tst.js:275:9:275:21 | arr.join(" ") | -| tst.js:275:9:275:11 | arr | tst.js:275:9:275:21 | arr.join(" ") | -| tst.js:300:10:300:33 | s().rep ... ]/g,'') | tst.js:300:10:300:33 | s().rep ... ]/g,'') | -| tst.js:301:10:301:32 | s().rep ... ]/g,'') | tst.js:301:10:301:32 | s().rep ... ]/g,'') | -| tst.js:302:10:302:34 | s().rep ... ]/g,'') | tst.js:302:10:302:34 | s().rep ... ]/g,'') | -| tst.js:303:10:303:34 | s().rep ... /g, '') | tst.js:303:10:303:34 | s().rep ... /g, '') | -| tst.js:309:10:318:3 | s().rep ... ;";\\n\\t}) | tst.js:309:10:318:3 | s().rep ... ;";\\n\\t}) | +subpaths #select | tst.js:243:9:243:31 | s().rep ... ]/g,'') | tst.js:243:9:243:31 | s().rep ... ]/g,'') | tst.js:243:9:243:31 | s().rep ... ]/g,'') | Cross-site scripting vulnerability as the output of $@ may contain double quotes when it reaches this attribute definition. | tst.js:243:9:243:31 | s().rep ... ]/g,'') | this final HTML sanitizer step | | tst.js:244:9:244:33 | s().rep ... /g, '') | tst.js:244:9:244:33 | s().rep ... /g, '') | tst.js:244:9:244:33 | s().rep ... /g, '') | Cross-site scripting vulnerability as the output of $@ may contain double quotes when it reaches this attribute definition. | tst.js:244:9:244:33 | s().rep ... /g, '') | this final HTML sanitizer step |