From d76916f9cec81ddb151edd9fb40a3c7c4df37eb0 Mon Sep 17 00:00:00 2001 From: tombolton Date: Tue, 28 Jun 2022 10:33:07 +0100 Subject: [PATCH] remove Xss from ATM --- .../adaptivethreatmodeling/XssATM.qll | 129 ------------------ .../modelbuilding/counting/CountXss.ql | 9 -- .../extraction/ExtractEndpointData.qll | 5 - .../extraction/ExtractEndpointMapping.ql | 3 - .../modelbuilding/extraction/Queries.qll | 4 - 5 files changed, 150 deletions(-) delete mode 100644 javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssATM.qll delete mode 100644 javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/counting/CountXss.ql diff --git a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssATM.qll b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssATM.qll deleted file mode 100644 index 71c75083b8b..00000000000 --- a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssATM.qll +++ /dev/null @@ -1,129 +0,0 @@ -/** - * For internal use only. - * - * Defines shared code used by the XSS boosted query. - */ - -private import semmle.javascript.heuristics.SyntacticHeuristics -private import semmle.javascript.security.dataflow.DomBasedXssCustomizations -import AdaptiveThreatModeling -import CoreKnowledge as CoreKnowledge -import StandardEndpointFilters as StandardEndpointFilters - -/** - * This module provides logic to filter candidate sinks to those which are likely XSS sinks. - */ -module SinkEndpointFilter { - private import javascript - private import DomBasedXss - - /** - * Provides a set of reasons why a given data flow node should be excluded as a sink candidate. - * - * If this predicate has no results for a sink candidate `n`, then we should treat `n` as an - * effective sink. - */ - string getAReasonSinkExcluded(DataFlow::Node sinkCandidate) { - result = StandardEndpointFilters::getAReasonSinkExcluded(sinkCandidate) - or - exists(DataFlow::CallNode call | sinkCandidate = call.getAnArgument() | - call.getCalleeName() = "setState" - ) and - result = "setState calls ought to be safe in react applications" - or - // Require XSS sink candidates to be (a) arguments to external library calls (possibly - // indirectly), or (b) heuristic sinks. - // - // Heuristic sinks are copied from the `HeuristicDomBasedXssSink` class defined within - // `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`. - // We can't reuse the class because importing that file would cause us to treat these - // heuristic sinks as known sinks. - not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(sinkCandidate) and - not ( - isAssignedToOrConcatenatedWith(sinkCandidate, "(?i)(html|innerhtml)") - or - isArgTo(sinkCandidate, "(?i)(html|render)") - or - sinkCandidate instanceof StringOps::HtmlConcatenationLeaf - or - isConcatenatedWithStrings("(?is).*<[a-z ]+.*", sinkCandidate, "(?s).*>.*") - or - // In addition to the heuristic sinks from `HeuristicDomBasedXssSink`, explicitly allow - // property writes like `elem.innerHTML = ` that may not be picked up as HTML - // concatenation leaves. - exists(DataFlow::PropWrite pw | - pw.getPropertyName().regexpMatch("(?i).*html*") and - pw.getRhs() = sinkCandidate - ) - ) and - result = "not a direct argument to a likely external library call or a heuristic sink" - } -} - -class DomBasedXssAtmConfig extends AtmConfig { - DomBasedXssAtmConfig() { this = "DomBasedXssATMConfig" } - - override predicate isKnownSource(DataFlow::Node source) { source instanceof DomBasedXss::Source } - - override predicate isKnownSink(DataFlow::Node sink) { sink instanceof DomBasedXss::Sink } - - override predicate isEffectiveSink(DataFlow::Node sinkCandidate) { - not exists(SinkEndpointFilter::getAReasonSinkExcluded(sinkCandidate)) - } - - override EndpointType getASinkEndpointType() { result instanceof XssSinkType } -} - -/** DEPRECATED: Alias for DomBasedXssAtmConfig */ -deprecated class DomBasedXssATMConfig = DomBasedXssAtmConfig; - -/** - * A taint-tracking configuration for reasoning about XSS vulnerabilities. - * - * This is largely a copy of the taint tracking configuration for the standard XSSThroughDom query, - * except additional ATM sinks have been added to the `isSink` predicate. - */ -class Configuration extends TaintTracking::Configuration { - Configuration() { this = "DomBasedXssATMConfiguration" } - - override predicate isSource(DataFlow::Node source) { source instanceof DomBasedXss::Source } - - override predicate isSink(DataFlow::Node sink) { - sink instanceof DomBasedXss::Sink or - any(DomBasedXssAtmConfig cfg).isEffectiveSink(sink) - } - - override predicate isSanitizer(DataFlow::Node node) { - super.isSanitizer(node) or - node instanceof DomBasedXss::Sanitizer - } - - override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) { - guard instanceof PrefixStringSanitizerActivated or - guard instanceof QuoteGuard or - guard instanceof ContainsHtmlGuard - } - - override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { - DomBasedXss::isOptionallySanitizedEdge(pred, succ) - } -} - -private import semmle.javascript.security.dataflow.Xss::Shared as Shared - -private class PrefixStringSanitizerActivated extends TaintTracking::SanitizerGuardNode, - DomBasedXss::PrefixStringSanitizer { - PrefixStringSanitizerActivated() { this = this } -} - -private class PrefixStringActivated extends DataFlow::FlowLabel, DomBasedXss::PrefixString { - PrefixStringActivated() { this = this } -} - -private class QuoteGuard extends TaintTracking::SanitizerGuardNode, Shared::QuoteGuard { - QuoteGuard() { this = this } -} - -private class ContainsHtmlGuard extends TaintTracking::SanitizerGuardNode, Shared::ContainsHtmlGuard { - ContainsHtmlGuard() { this = this } -} diff --git a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/counting/CountXss.ql b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/counting/CountXss.ql deleted file mode 100644 index 281e6c16741..00000000000 --- a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/counting/CountXss.ql +++ /dev/null @@ -1,9 +0,0 @@ -/* - * For internal use only. - * - * - * Count the number of sinks and alerts for the `DomBasedXss` security query. - */ - -import semmle.javascript.security.dataflow.DomBasedXssQuery -import CountAlertsAndSinks diff --git a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointData.qll b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointData.qll index 57217cda350..ce3fff24c1f 100644 --- a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointData.qll +++ b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointData.qll @@ -17,7 +17,6 @@ import experimental.adaptivethreatmodeling.FilteringReasons import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionATM import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionATM import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathATM -import experimental.adaptivethreatmodeling.XssATM as XssATM import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomATM import Labels import NoFeaturizationRestrictionsConfig @@ -32,8 +31,6 @@ AtmConfig getAtmCfg(Query query) { or query instanceof TaintedPathQuery and result instanceof TaintedPathATM::TaintedPathAtmConfig or - query instanceof XssQuery and result instanceof XssATM::DomBasedXssAtmConfig - or query instanceof XssThroughDomQuery and result instanceof XssThroughDomATM::XssThroughDomAtmConfig } @@ -48,8 +45,6 @@ DataFlow::Configuration getDataFlowCfg(Query query) { or query instanceof TaintedPathQuery and result instanceof TaintedPathATM::Configuration or - query instanceof XssQuery and result instanceof XssATM::Configuration - or query instanceof XssThroughDomQuery and result instanceof XssThroughDomATM::Configuration } diff --git a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql index eff296cd840..cfc95ea64fa 100644 --- a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql +++ b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql @@ -7,7 +7,6 @@ import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionATM import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionATM import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathATM -import experimental.adaptivethreatmodeling.XssATM as XssATM import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomATM import experimental.adaptivethreatmodeling.AdaptiveThreatModeling @@ -23,8 +22,6 @@ where queryName = "TaintedPath" and c instanceof TaintedPathATM::TaintedPathAtmConfig or - queryName = "Xss" and c instanceof XssATM::DomBasedXssAtmConfig - or queryName = "XssThroughDom" and c instanceof XssThroughDomATM::XssThroughDomAtmConfig ) and e = c.getASinkEndpointType() diff --git a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/Queries.qll b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/Queries.qll index eba5a8a2829..977013e748c 100644 --- a/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/Queries.qll +++ b/javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/Queries.qll @@ -30,10 +30,6 @@ class TaintedPathQuery extends Query, TTaintedPathQuery { override string getName() { result = "TaintedPath" } } -class XssQuery extends Query, TXssQuery { - override string getName() { result = "Xss" } -} - class XssThroughDomQuery extends Query, TXssThroughDomQuery { override string getName() { result = "XssThroughDom" } }