diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll index d89ab06ed829..a1d9dd86c400 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll @@ -148,6 +148,19 @@ module SourceSinkInterpretationInput implements ) } + predicate barrierElement( + Element n, string output, string kind, Public::Provenance provenance, string model + ) { + none() + } + + predicate barrierGuardElement( + Element n, string input, Public::AcceptingValue acceptingvalue, string kind, + Public::Provenance provenance, string model + ) { + none() + } + private newtype TInterpretNode = TElement_(Element n) or TNode_(Node n) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll index 3ee16f21489c..842c28ac75b0 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll @@ -235,6 +235,19 @@ module SourceSinkInterpretationInput implements ) } + predicate barrierElement( + Element n, string output, string kind, Public::Provenance provenance, string model + ) { + none() + } + + predicate barrierGuardElement( + Element n, string input, Public::AcceptingValue acceptingvalue, string kind, + Public::Provenance provenance, string model + ) { + none() + } + class SourceOrSinkElement = Element; private newtype TInterpretNode = diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index f12c9e6eeb1b..41ded04634be 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -163,6 +163,19 @@ module SourceSinkInterpretationInput implements ) } + predicate barrierElement( + Element n, string output, string kind, Public::Provenance provenance, string model + ) { + none() + } + + predicate barrierGuardElement( + Element n, string input, Public::AcceptingValue acceptingvalue, string kind, + Public::Provenance provenance, string model + ) { + none() + } + // Note that due to embedding, which is currently implemented via some // Methods having multiple qualified names, a given Method is liable to have // more than one SourceOrSinkElement, one for each of the names it claims. diff --git a/java/ql/lib/ext/empty.model.yml b/java/ql/lib/ext/empty.model.yml index 43028e7cc14e..a0ea3cf753ba 100644 --- a/java/ql/lib/ext/empty.model.yml +++ b/java/ql/lib/ext/empty.model.yml @@ -13,6 +13,14 @@ extensions: pack: codeql/java-all extensible: summaryModel data: [] + - addsTo: + pack: codeql/java-all + extensible: barrierModel + data: [] + - addsTo: + pack: codeql/java-all + extensible: barrierGuardModel + data: [] - addsTo: pack: codeql/java-all extensible: neutralModel diff --git a/java/ql/lib/ext/hudson.model.yml b/java/ql/lib/ext/hudson.model.yml index eda30b6a0ff7..0dfff091fcd4 100644 --- a/java/ql/lib/ext/hudson.model.yml +++ b/java/ql/lib/ext/hudson.model.yml @@ -50,6 +50,12 @@ extensions: - ["hudson", "FilePath", False, "readToString", "", "", "ReturnValue", "file", "manual"] - ["hudson", "Plugin", True, "configure", "", "", "Parameter", "remote", "manual"] - ["hudson", "Plugin", True, "newInstance", "", "", "Parameter", "remote", "manual"] + - addsTo: + pack: codeql/java-all + extensible: barrierModel + data: + - ["hudson", "Util", True, "escape", "(String)", "", "ReturnValue", "html-injection", "manual"] + # Not including xmlEscape because it only accounts for >, <, and &. It does not account for ", or ', which makes it an incomplete XSS sanitizer. - addsTo: pack: codeql/java-all extensible: summaryModel diff --git a/java/ql/lib/ext/java.io.model.yml b/java/ql/lib/ext/java.io.model.yml index 3582e2b78ac6..07e39c9e12f7 100644 --- a/java/ql/lib/ext/java.io.model.yml +++ b/java/ql/lib/ext/java.io.model.yml @@ -162,3 +162,8 @@ extensions: extensible: sourceModel data: - ["java.io", "FileInputStream", True, "FileInputStream", "", "", "Argument[this]", "file", "manual"] + - addsTo: + pack: codeql/java-all + extensible: barrierModel + data: + - ["java.io", "File", True, "getName", "()", "", "ReturnValue", "path-injection", "manual"] diff --git a/java/ql/lib/ext/java.net.model.yml b/java/ql/lib/ext/java.net.model.yml index 084fce7bbc4b..e69db468a4a4 100644 --- a/java/ql/lib/ext/java.net.model.yml +++ b/java/ql/lib/ext/java.net.model.yml @@ -34,6 +34,11 @@ extensions: - ["java.net", "URLClassLoader", False, "URLClassLoader", "(URL[],ClassLoader)", "", "Argument[0]", "request-forgery", "manual"] - ["java.net", "URLClassLoader", False, "URLClassLoader", "(URL[])", "", "Argument[0]", "request-forgery", "manual"] - ["java.net", "PasswordAuthentication", False, "PasswordAuthentication", "(String,char[])", "", "Argument[0]", "credentials-username", "hq-generated"] + - addsTo: + pack: codeql/java-all + extensible: barrierGuardModel + data: + - ["java.net", "URI", True, "isAbsolute", "()", "", "Argument[this]", "false", "request-forgery", "manual"] - addsTo: pack: codeql/java-all extensible: summaryModel diff --git a/java/ql/lib/ext/java.util.regex.model.yml b/java/ql/lib/ext/java.util.regex.model.yml index 4f0776e59bd2..20269a271b55 100644 --- a/java/ql/lib/ext/java.util.regex.model.yml +++ b/java/ql/lib/ext/java.util.regex.model.yml @@ -12,6 +12,11 @@ extensions: - ["java.util.regex", "Pattern", False, "split", "(CharSequence)", "", "Argument[this]", "regex-use[0]", "manual"] - ["java.util.regex", "Pattern", False, "split", "(CharSequence,int)", "", "Argument[this]", "regex-use[0]", "manual"] - ["java.util.regex", "Pattern", False, "splitAsStream", "(CharSequence)", "", "Argument[this]", "regex-use[0]", "manual"] + - addsTo: + pack: codeql/java-all + extensible: barrierModel + data: + - ["java.util.regex", "Pattern", False, "quote", "(String)", "", "ReturnValue", "regex-use", "manual"] - addsTo: pack: codeql/java-all extensible: summaryModel diff --git a/java/ql/lib/ext/org.owasp.esapi.model.yml b/java/ql/lib/ext/org.owasp.esapi.model.yml index 30578debe580..70890d7e03b3 100644 --- a/java/ql/lib/ext/org.owasp.esapi.model.yml +++ b/java/ql/lib/ext/org.owasp.esapi.model.yml @@ -1,6 +1,42 @@ extensions: + - addsTo: + pack: codeql/java-all + extensible: barrierGuardModel + data: + - ["org.owasp.esapi", "Validator", true, "isValidCreditCard", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidDate", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidDirectoryPath", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidDouble", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidFileContent", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidFileName", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidInput", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidInteger", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidListItem", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidNumber", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidPrintable", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidRedirectLocation", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidSafeHTML", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "isValidURI", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"] + - addsTo: + pack: codeql/java-all + extensible: barrierModel + data: + - ["org.owasp.esapi", "Validator", true, "getValidCreditCard", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidDate", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidDirectoryPath", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidDouble", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidFileContent", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidFileName", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidInput", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidInteger", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidListItem", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidNumber", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidPrintable", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidRedirectLocation", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidSafeHTML", "", "", "ReturnValue", "trust-boundary-violation", "manual"] + - ["org.owasp.esapi", "Validator", true, "getValidURI", "", "", "ReturnValue", "trust-boundary-violation", "manual"] - addsTo: pack: codeql/java-all extensible: summaryModel data: - - ["org.owasp.esapi", "Encoder", true, "encodeForHTML", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"] \ No newline at end of file + - ["org.owasp.esapi", "Encoder", true, "encodeForHTML", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"] diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index d1849df0f3ec..e612239a76ec 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -91,6 +91,7 @@ module; import java private import semmle.code.java.dataflow.DataFlow::DataFlow +private import semmle.code.java.controlflow.Guards private import FlowSummary as FlowSummary private import internal.DataFlowPrivate private import internal.FlowSummaryImpl @@ -174,6 +175,24 @@ predicate sinkModel( ) } +/** Holds if a barrier model exists for the given parameters. */ +predicate barrierModel( + string package, string type, boolean subtypes, string name, string signature, string ext, + string output, string kind, string provenance, QlBuiltins::ExtensionId madId +) { + Extensions::barrierModel(package, type, subtypes, name, signature, ext, output, kind, provenance, + madId) +} + +/** Holds if a barrier guard model exists for the given parameters. */ +predicate barrierGuardModel( + string package, string type, boolean subtypes, string name, string signature, string ext, + string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId +) { + Extensions::barrierGuardModel(package, type, subtypes, name, signature, ext, input, + acceptingvalue, kind, provenance, madId) +} + /** Holds if a summary model exists for the given parameters. */ predicate summaryModel( string package, string type, boolean subtypes, string name, string signature, string ext, @@ -234,6 +253,7 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) { "Summary: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " + ext + "; " + input + "; " + output + "; " + kind + "; " + provenance ) + //TODO: possibly barrier models? } /** Holds if a neutral model exists for the given parameters. */ @@ -292,6 +312,7 @@ predicate modelCoverage(string package, int pkgs, string kind, string part, int summaryModel(subpkg, type, subtypes, name, signature, ext, input, output, kind, provenance, _) ) + // TODO: possibly barrier models? ) } @@ -303,7 +324,9 @@ module ModelValidation { summaryModel(_, _, _, _, _, _, path, _, _, _, _) or summaryModel(_, _, _, _, _, _, _, path, _, _, _) or sinkModel(_, _, _, _, _, _, path, _, _, _) or - sourceModel(_, _, _, _, _, _, path, _, _, _) + sourceModel(_, _, _, _, _, _, path, _, _, _) or + barrierModel(_, _, _, _, _, _, path, _, _, _) or + barrierGuardModel(_, _, _, _, _, _, path, _, _, _, _) } private module MkAccessPath = AccessPathSyntax::AccessPath; @@ -316,6 +339,8 @@ module ModelValidation { exists(string pred, AccessPath input, AccessPathToken part | sinkModel(_, _, _, _, _, _, input, _, _, _) and pred = "sink" or + barrierGuardModel(_, _, _, _, _, _, input, _, _, _, _) and pred = "barrier guard" + or summaryModel(_, _, _, _, _, _, input, _, _, _, _) and pred = "summary" | ( @@ -338,6 +363,8 @@ module ModelValidation { exists(string pred, AccessPath output, AccessPathToken part | sourceModel(_, _, _, _, _, _, output, _, _, _) and pred = "source" or + barrierModel(_, _, _, _, _, _, output, _, _, _) and pred = "barrier" + or summaryModel(_, _, _, _, _, _, _, output, _, _, _) and pred = "summary" | ( @@ -355,7 +382,13 @@ module ModelValidation { private module KindValConfig implements SharedModelVal::KindValidationConfigSig { predicate summaryKind(string kind) { summaryModel(_, _, _, _, _, _, _, _, kind, _, _) } - predicate sinkKind(string kind) { sinkModel(_, _, _, _, _, _, _, kind, _, _) } + predicate sinkKind(string kind) { + sinkModel(_, _, _, _, _, _, _, kind, _, _) + or + barrierModel(_, _, _, _, _, _, _, kind, _, _) + or + barrierGuardModel(_, _, _, _, _, _, _, _, kind, _, _) + } predicate sourceKind(string kind) { sourceModel(_, _, _, _, _, _, _, kind, _, _) } @@ -373,6 +406,11 @@ module ModelValidation { or sinkModel(package, type, _, name, signature, ext, _, _, provenance, _) and pred = "sink" or + barrierModel(package, type, _, name, signature, ext, _, _, provenance, _) and pred = "barrier" + or + barrierGuardModel(package, type, _, name, signature, ext, _, _, _, provenance, _) and + pred = "barrier guard" + or summaryModel(package, type, _, name, signature, ext, _, _, _, provenance, _) and pred = "summary" or @@ -398,6 +436,14 @@ module ModelValidation { invalidProvenance(provenance) and result = "Unrecognized provenance description \"" + provenance + "\" in " + pred + " model." ) + or + exists(string acceptingvalue | + barrierGuardModel(_, _, _, _, _, _, _, acceptingvalue, _, _, _) and + invalidAcceptingValue(acceptingvalue) and + result = + "Unrecognized accepting value description \"" + acceptingvalue + + "\" in barrier guard model." + ) } /** Holds if some row in a MaD flow model appears to contain typos. */ @@ -418,6 +464,10 @@ private predicate elementSpec( or sinkModel(package, type, subtypes, name, signature, ext, _, _, _, _) or + barrierModel(package, type, subtypes, name, signature, ext, _, _, _, _) + or + barrierGuardModel(package, type, subtypes, name, signature, ext, _, _, _, _, _) + or summaryModel(package, type, subtypes, name, signature, ext, _, _, _, _, _) or neutralModel(package, type, name, signature, _, _) and ext = "" and subtypes = true @@ -578,6 +628,53 @@ private module Cached { isSinkNode(n, kind, model) and n.asNode() = node ) } + + private newtype TKindModelPair = + TMkPair(string kind, string model) { isBarrierGuardNode(_, _, kind, model) } + + private GuardValue convertAcceptingValue(AcceptingValue av) { + av.isTrue() and result.asBooleanValue() = true + or + av.isFalse() and result.asBooleanValue() = false + or + av.isNoException() and result.getDualValue().isThrowsException() + or + av.isZero() and result.asIntValue() = 0 + or + av.isNotZero() and result.getDualValue().asIntValue() = 0 + or + av.isNull() and result.isNullValue() + or + av.isNotNull() and result.isNonNullValue() + } + + private predicate barrierGuardChecks(Guard g, Expr e, GuardValue gv, TKindModelPair kmp) { + exists( + SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingvalue, string kind, + string model + | + isBarrierGuardNode(n, acceptingvalue, kind, model) and + n.asNode().asExpr() = e and + kmp = TMkPair(kind, model) and + gv = convertAcceptingValue(acceptingvalue) + | + g.(Call).getAnArgument() = e or g.(MethodCall).getQualifier() = e + ) + } + + /** + * Holds if `node` is specified as a barrier with the given kind in a MaD flow + * model. + */ + cached + predicate barrierNode(Node node, string kind, string model) { + exists(SourceSinkInterpretationInput::InterpretNode n | + isBarrierNode(n, kind, model) and n.asNode() = node + ) + or + ParameterizedBarrierGuard::getABarrierNode(TMkPair(kind, + model)) = node + } } import Cached @@ -594,6 +691,12 @@ predicate sourceNode(Node node, string kind) { sourceNode(node, kind, _) } */ predicate sinkNode(Node node, string kind) { sinkNode(node, kind, _) } +/** + * Holds if `node` is specified as a barrier with the given kind in a MaD flow + * model. + */ +predicate barrierNode(Node node, string kind) { barrierNode(node, kind, _) } + // adapter class for converting Mad summaries to `SummarizedCallable`s private class SummarizedCallableAdapter extends SummarizedCallable { SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _, _) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll index d96834a55331..e2e80c293ef0 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -420,3 +420,31 @@ module BarrierGuard { /** Gets a node that is safely guarded by the given guard check. */ Node getABarrierNode() { result = BarrierGuardValue::getABarrierNode() } } + +bindingset[this] +private signature class ParamSig; + +private module WithParam { + /** + * Holds if the guard `g` validates the expression `e` upon evaluating to `gv`. + * + * The expression `e` is expected to be a syntactic part of the guard `g`. + * For example, the guard `g` might be a call `isSafe(x)` and the expression `e` + * the argument `x`. + */ + signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv, P param); +} + +/** + * Provides a set of barrier nodes for a guard that validates an expression. + * + * This is expected to be used in `isBarrier`/`isSanitizer` definitions + * in data flow and taint tracking. + */ +module ParameterizedBarrierGuard::guardChecksSig/4 guardChecks> { + /** Gets a node that is safely guarded by the given guard check. */ + Node getABarrierNode(P param) { + SsaFlow::asNode(result) = + SsaImpl::DataFlowIntegration::ParameterizedBarrierGuard::getABarrierNode(param) + } +} diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll b/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll index 32b5d289e28c..5386d916e240 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/ExternalFlowExtensions.qll @@ -20,6 +20,22 @@ extensible predicate sinkModel( string input, string kind, string provenance, QlBuiltins::ExtensionId madId ); +/** + * Holds if a barrier model exists for the given parameters. + */ +extensible predicate barrierModel( + string package, string type, boolean subtypes, string name, string signature, string ext, + string output, string kind, string provenance, QlBuiltins::ExtensionId madId +); + +/** + * Holds if a barrier guard model exists for the given parameters. + */ +extensible predicate barrierGuardModel( + string package, string type, boolean subtypes, string name, string signature, string ext, + string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId +); + /** * Holds if a summary model exists for the given parameters. */ diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll index 8dc28ea2f601..19d9a33328b8 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll @@ -158,7 +158,9 @@ private predicate relatedArgSpec(Callable c, string spec) { summaryModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _, _) or summaryModel(namespace, type, subtypes, name, signature, ext, _, spec, _, _, _) or sourceModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or - sinkModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) + sinkModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or + barrierModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or + barrierGuardModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _, _) | c = interpretElement(namespace, type, subtypes, name, signature, ext, _) ) @@ -259,6 +261,45 @@ module SourceSinkInterpretationInput implements ) } + predicate barrierElement( + Element e, string output, string kind, Public::Provenance provenance, string model + ) { + exists( + string namespace, string type, boolean subtypes, string name, string signature, string ext, + SourceOrSinkElement baseBarrier, string originalOutput, QlBuiltins::ExtensionId madId + | + barrierModel(namespace, type, subtypes, name, signature, ext, originalOutput, kind, + provenance, madId) and + model = "MaD:" + madId.toString() and + baseBarrier = interpretElement(namespace, type, subtypes, name, signature, ext, _) and + ( + e = baseBarrier and output = originalOutput + or + correspondingKotlinParameterDefaultsArgSpec(baseBarrier, e, originalOutput, output) + ) + ) + } + + predicate barrierGuardElement( + Element e, string output, Public::AcceptingValue acceptingvalue, string kind, + Public::Provenance provenance, string model + ) { + exists( + string namespace, string type, boolean subtypes, string name, string signature, string ext, + SourceOrSinkElement baseBarrier, string originalOutput, QlBuiltins::ExtensionId madId + | + barrierGuardModel(namespace, type, subtypes, name, signature, ext, originalOutput, + acceptingvalue, kind, provenance, madId) and + model = "MaD:" + madId.toString() and + baseBarrier = interpretElement(namespace, type, subtypes, name, signature, ext, _) and + ( + e = baseBarrier and output = originalOutput + or + correspondingKotlinParameterDefaultsArgSpec(baseBarrier, e, originalOutput, output) + ) + ) + } + class SourceOrSinkElement = Element; private newtype TInterpretNode = diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index 323f14f550ff..bafb16d6ab53 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll @@ -588,6 +588,36 @@ private module Cached { predicate getABarrierNode = getABarrierNodeImpl/0; } + + bindingset[this] + private signature class ParamSig; + + private module WithParam { + signature predicate guardChecksSig(Guards::Guard g, Expr e, Guards::GuardValue gv, P param); + } + + cached // nothing is actually cached + module ParameterizedBarrierGuard::guardChecksSig/4 guardChecks> { + private predicate guardChecksAdjTypes( + Guards::Guards_v3::Guard g, Expr e, Guards::GuardValue gv, P param + ) { + guardChecks(g, e, gv, param) + } + + private predicate guardChecksWithWrappers( + DataFlowIntegrationInput::Guard g, Definition def, Guards::GuardValue val, P param + ) { + Guards::Guards_v3::ParameterizedValidationWrapper::guardChecksDef(g, + def, val, param) + } + + private Node getABarrierNodeImpl(P param) { + result = + DataFlowIntegrationImpl::BarrierGuardDefWithState::getABarrierNode(param) + } + + predicate getABarrierNode = getABarrierNodeImpl/1; + } } cached diff --git a/java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll b/java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll index 44752f94576b..86b659289171 100644 --- a/java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll +++ b/java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll @@ -14,14 +14,3 @@ class HudsonWebMethod extends Method { this.getDeclaringType().getASourceSupertype*().hasQualifiedName("hudson.model", "Descriptor") } } - -private class HudsonUtilXssSanitizer extends XssSanitizer { - HudsonUtilXssSanitizer() { - this.asExpr() - .(MethodCall) - .getMethod() - // Not including xmlEscape because it only accounts for >, <, and &. - // It does not account for ", or ', which makes it an incomplete XSS sanitizer. - .hasQualifiedName("hudson", "Util", "escape") - } -} diff --git a/java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll b/java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll deleted file mode 100644 index fe95cd0d39d3..000000000000 --- a/java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll +++ /dev/null @@ -1,42 +0,0 @@ -/** Classes and predicates for reasoning about the `owasp.easpi` package. */ -overlay[local?] -module; - -import java - -/** - * The `org.owasp.esapi.Validator` interface. - */ -class EsapiValidator extends RefType { - EsapiValidator() { this.hasQualifiedName("org.owasp.esapi", "Validator") } -} - -/** - * The methods of `org.owasp.esapi.Validator` which validate data. - */ -class EsapiIsValidMethod extends Method { - EsapiIsValidMethod() { - this.getDeclaringType() instanceof EsapiValidator and - this.hasName([ - "isValidCreditCard", "isValidDate", "isValidDirectoryPath", "isValidDouble", - "isValidFileContent", "isValidFileName", "isValidInput", "isValidInteger", - "isValidListItem", "isValidNumber", "isValidPrintable", "isValidRedirectLocation", - "isValidSafeHTML", "isValidURI" - ]) - } -} - -/** - * The methods of `org.owasp.esapi.Validator` which return validated data. - */ -class EsapiGetValidMethod extends Method { - EsapiGetValidMethod() { - this.getDeclaringType() instanceof EsapiValidator and - this.hasName([ - "getValidCreditCard", "getValidDate", "getValidDirectoryPath", "getValidDouble", - "getValidFileContent", "getValidFileName", "getValidInput", "getValidInteger", - "getValidListItem", "getValidNumber", "getValidPrintable", "getValidRedirectLocation", - "getValidSafeHTML", "getValidURI" - ]) - } -} diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll index da6f242bde52..2018004a3fb5 100644 --- a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll +++ b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll @@ -4,6 +4,7 @@ module; import java private import semmle.code.java.controlflow.Guards +private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.dataflow.SSA private import semmle.code.java.frameworks.kotlin.IO @@ -288,19 +289,8 @@ private Method getSourceMethod(Method m) { result = m } -/** - * A sanitizer that protects against path injection vulnerabilities - * by extracting the final component of the user provided path. - * - * TODO: convert this class to models-as-data if sanitizer support is added - */ -private class FileGetNameSanitizer extends PathInjectionSanitizer { - FileGetNameSanitizer() { - exists(MethodCall mc | - mc.getMethod().hasQualifiedName("java.io", "File", "getName") and - this.asExpr() = mc - ) - } +private class DefaultPathInjectionSanitizer extends PathInjectionSanitizer { + DefaultPathInjectionSanitizer() { barrierNode(this, "path-injection") } } /** Holds if `g` is a guard that checks for `..` components. */ diff --git a/java/ql/lib/semmle/code/java/security/RequestForgery.qll b/java/ql/lib/semmle/code/java/security/RequestForgery.qll index 9e3dec003579..690e4f9315b9 100644 --- a/java/ql/lib/semmle/code/java/security/RequestForgery.qll +++ b/java/ql/lib/semmle/code/java/security/RequestForgery.qll @@ -118,25 +118,8 @@ private class ContainsUrlSanitizer extends RequestForgerySanitizer { } } -/** - * A check that the URL is relative, and therefore safe for URL redirects. - */ -private predicate isRelativeUrlSanitizer(Guard guard, Expr e, boolean branch) { - guard = - any(MethodCall call | - call.getMethod().hasQualifiedName("java.net", "URI", "isAbsolute") and - e = call.getQualifier() and - branch = false - ) -} - -/** - * A check that the URL is relative, and therefore safe for URL redirects. - */ -private class RelativeUrlSanitizer extends RequestForgerySanitizer { - RelativeUrlSanitizer() { - this = DataFlow::BarrierGuard::getABarrierNode() - } +private class DefaultRequestForgerySanitizer extends RequestForgerySanitizer { + DefaultRequestForgerySanitizer() { barrierNode(this, "request-forgery") } } /** diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index b2f49834b5ab..477aeb48b64e 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -5,7 +5,6 @@ private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.controlflow.Guards private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources -private import semmle.code.java.frameworks.owasp.Esapi private import semmle.code.java.security.Sanitizers /** @@ -28,25 +27,8 @@ class TrustBoundaryViolationSink extends DataFlow::Node { */ abstract class TrustBoundaryValidationSanitizer extends DataFlow::Node { } -/** - * A node validated by an OWASP ESAPI validation method. - */ -private class EsapiValidatedInputSanitizer extends TrustBoundaryValidationSanitizer { - EsapiValidatedInputSanitizer() { - this = DataFlow::BarrierGuard::getABarrierNode() or - this.asExpr().(MethodCall).getMethod() instanceof EsapiGetValidMethod - } -} - -/** - * Holds if `g` is a guard that checks that `e` is valid data according to an OWASP ESAPI validation method. - */ -private predicate esapiIsValidData(Guard g, Expr e, boolean branch) { - branch = true and - exists(MethodCall ma | ma.getMethod() instanceof EsapiIsValidMethod | - g = ma and - e = ma.getArgument(1) - ) +private class DefaultTrustBoundaryValidationSanitizer extends TrustBoundaryValidationSanitizer { + DefaultTrustBoundaryValidationSanitizer() { barrierNode(this, "trust-boundary-violation") } } /** diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll index 990371cc8cfe..c131f868f36c 100644 --- a/java/ql/lib/semmle/code/java/security/XSS.qll +++ b/java/ql/lib/semmle/code/java/security/XSS.qll @@ -54,12 +54,24 @@ private class DefaultXssSink extends XssSink { } } -/** A default sanitizer that considers numeric and boolean typed data safe for writing to output. */ private class DefaultXssSanitizer extends XssSanitizer { - DefaultXssSanitizer() { + DefaultXssSanitizer() { barrierNode(this, ["html-injection", "js-injection"]) } +} + +/** A sanitizer that considers numeric and boolean typed data safe for writing to output. */ +private class PrimitiveSanitizer extends XssSanitizer { + PrimitiveSanitizer() { this.getType() instanceof NumericType or - this.getType() instanceof BooleanType or - // Match `org.springframework.web.util.HtmlUtils.htmlEscape` and possibly other methods like it. + this.getType() instanceof BooleanType + } +} + +/** + * A call to `org.springframework.web.util.HtmlUtils.htmlEscape`, or possibly + * other methods like it, considered as a sanitizer for XSS. + */ +private class HtmlEscapeXssSanitizer extends XssSanitizer { + HtmlEscapeXssSanitizer() { this.asExpr().(MethodCall).getMethod().getName().regexpMatch("(?i)html_?escape.*") } } diff --git a/java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll b/java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll index eb27ec873752..d91b411b7978 100644 --- a/java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll +++ b/java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll @@ -21,17 +21,8 @@ private class DefaultRegexInjectionSink extends RegexInjectionSink { } } -/** - * A call to the `Pattern.quote` method, which gives metacharacters or escape sequences - * no special meaning. - */ -private class PatternQuoteCall extends RegexInjectionSanitizer { - PatternQuoteCall() { - exists(MethodCall ma, Method m | m = ma.getMethod() | - ma.getArgument(0) = this.asExpr() and - m instanceof PatternQuoteMethod - ) - } +private class DefaultRegexInjectionSanitizer extends RegexInjectionSanitizer { + DefaultRegexInjectionSanitizer() { barrierNode(this, "regex-use") } } /** diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll index 80ec45a3cf17..ee2cd8f57bfe 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll @@ -344,6 +344,26 @@ private predicate sinkModel(string type, string path, string kind, string model) ) } +/** Holds if a barrier model exists for the given parameters. */ +private predicate barrierModel(string type, string path, string kind, string model) { + // No deprecation adapter for barrier models, they were not around back then. + exists(QlBuiltins::ExtensionId madId | + Extensions::barrierModel(type, path, kind, madId) and + model = "MaD:" + madId.toString() + ) +} + +/** Holds if a barrier guard model exists for the given parameters. */ +private predicate barrierGuardModel( + string type, string path, string branch, string kind, string model +) { + // No deprecation adapter for barrier models, they were not around back then. + exists(QlBuiltins::ExtensionId madId | + Extensions::barrierGuardModel(type, path, branch, kind, madId) and + model = "MaD:" + madId.toString() + ) +} + /** Holds if a summary model `row` exists for the given parameters. */ private predicate summaryModel( string type, string path, string input, string output, string kind, string model @@ -400,6 +420,8 @@ predicate isRelevantType(string type) { ( sourceModel(type, _, _, _) or sinkModel(type, _, _, _) or + barrierModel(type, _, _, _) or + barrierGuardModel(type, _, _, _, _) or summaryModel(type, _, _, _, _, _) or typeModel(_, type, _) ) and @@ -427,6 +449,8 @@ predicate isRelevantFullPath(string type, string path) { ( sourceModel(type, path, _, _) or sinkModel(type, path, _, _) or + barrierModel(type, path, _, _) or + barrierGuardModel(type, path, _, _, _) or summaryModel(type, path, _, _, _, _) or typeModel(_, type, path) ) @@ -745,6 +769,32 @@ module ModelOutput { ) } + /** + * Holds if a barrier model contributed `barrier` with the given `kind`. + */ + cached + API::Node getABarrierNode(string kind, string model) { + exists(string type, string path | + barrierModel(type, path, kind, model) and + result = getNodeFromPath(type, path) + ) + } + + /** + * Holds if a barrier model contributed `barrier` with the given `kind`. + */ + cached + API::Node getABarrierGuardNode(string kind, boolean branch, string model) { + exists(string type, string path, string branch_str | + branch = true and branch_str = "true" + or + branch = false and branch_str = "false" + | + barrierGuardModel(type, path, branch_str, kind, model) and + result = getNodeFromPath(type, path) + ) + } + /** * Holds if a relevant summary exists for these parameters. */ @@ -787,15 +837,27 @@ module ModelOutput { private import codeql.mad.ModelValidation as SharedModelVal /** - * Holds if a CSV source model contributed `source` with the given `kind`. + * Holds if an external model contributed `source` with the given `kind`. */ API::Node getASourceNode(string kind) { result = getASourceNode(kind, _) } /** - * Holds if a CSV sink model contributed `sink` with the given `kind`. + * Holds if an external model contributed `sink` with the given `kind`. */ API::Node getASinkNode(string kind) { result = getASinkNode(kind, _) } + /** + * Holds if an external model contributed `barrier` with the given `kind`. + */ + API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) } + + /** + * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`. + */ + API::Node getABarrierGuardNode(string kind, boolean branch) { + result = getABarrierGuardNode(kind, branch, _) + } + private module KindValConfig implements SharedModelVal::KindValidationConfigSig { predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind, _) } diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll index 3f38c498f324..2a644aabb95d 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -20,6 +20,26 @@ extensible predicate sourceModel( */ extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId); +/** + * Holds if the value at `(type, path)` should be seen as a barrier + * of the given `kind` and `madId` is the data extension row number. + */ +extensible predicate barrierModel( + string type, string path, string kind, QlBuiltins::ExtensionId madId +); + +/** + * Holds if the value at `(type, path)` should be seen as a barrier guard + * of the given `kind` and `madId` is the data extension row number. + * `path` is assumed to lead to a parameter of a call (possibly `self`), and + * the call is guarding the parameter. + * `branch` is either `true` or `false`, indicating which branch of the guard + * is protecting the parameter. + */ +extensible predicate barrierGuardModel( + string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId +); + /** * Holds if in calls to `(type, path)`, the value referred to by `input` * can flow to the value referred to by `output` and `madId` is the data diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/empty.model.yml b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/empty.model.yml index 12f83f71e55b..6542a1194cab 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/empty.model.yml +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/empty.model.yml @@ -16,6 +16,16 @@ extensions: extensible: summaryModel data: [] + - addsTo: + pack: codeql/javascript-all + extensible: barrierModel + data: [] + + - addsTo: + pack: codeql/javascript-all + extensible: barrierGuardModel + data: [] + - addsTo: pack: codeql/javascript-all extensible: neutralModel diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll index ffecbcba57ac..65e03963379a 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll @@ -600,15 +600,64 @@ signature predicate guardChecksSig(GuardNode g, ControlFlowNode node, boolean br module BarrierGuard { /** Gets a node that is safely guarded by the given guard check. */ ExprNode getABarrierNode() { + result = ParameterizedBarrierGuard::getABarrierNode(_) + } + + private predicate extendedGuardChecks(GuardNode g, ControlFlowNode node, boolean branch, Unit u) { + guardChecks(g, node, branch) + } +} + +bindingset[this] +private signature class ParamSig; + +private module WithParam { + signature predicate guardChecksSig(GuardNode g, ControlFlowNode node, boolean branch, P param); +} + +/** + * Provides a set of barrier nodes for a guard that validates a node. + * + * This is expected to be used in `isBarrier`/`isSanitizer` definitions + * in data flow and taint tracking. + */ +module ParameterizedBarrierGuard::guardChecksSig/4 guardChecks> { + /** Gets a node that is safely guarded by the given guard check with parameter `param`. */ + ExprNode getABarrierNode(P param) { exists(GuardNode g, EssaDefinition def, ControlFlowNode node, boolean branch | AdjacentUses::useOfDef(def, node) and - guardChecks(g, node, branch) and + guardChecks(g, node, branch, param) and AdjacentUses::useOfDef(def, result.asCfgNode()) and g.controlsBlock(result.asCfgNode().getBasicBlock(), branch) ) } } +/** + * Provides a set of barrier nodes for a guard that validates a node as described by an external predicate. + * + * This is expected to be used in `isBarrier`/`isSanitizer` definitions + * in data flow and taint tracking. + */ +module ExternalBarrierGuard { + private import semmle.python.ApiGraphs + + private predicate guardCheck(GuardNode g, ControlFlowNode node, boolean branch, string kind) { + exists(API::CallNode call, API::Node parameter | + parameter = call.getAParameter() and + parameter = ModelOutput::getABarrierGuardNode(kind, branch) + | + g = call.asCfgNode() and + node = parameter.asSink().asCfgNode() + ) + } + + /** Gets a node that is an external barrier of the given kind. */ + ExprNode getAnExternalBarrierNode(string kind) { + result = ParameterizedBarrierGuard::getABarrierNode(kind) + } +} + /** * Algebraic datatype for tracking data content associated with values. * Content can be collection elements or object attributes. diff --git a/python/ql/lib/semmle/python/frameworks/Django.model.yml b/python/ql/lib/semmle/python/frameworks/Django.model.yml new file mode 100644 index 000000000000..4e472af6c117 --- /dev/null +++ b/python/ql/lib/semmle/python/frameworks/Django.model.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/python-all + extensible: barrierGuardModel + data: + - ['django', 'Member[utils].Member[http].Member[url_has_allowed_host_and_scheme].Argument[0,url:]', "true", 'url-redirection'] diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll index 4aa5776ad54b..ee0ed4a84dd0 100644 --- a/python/ql/lib/semmle/python/frameworks/Django.qll +++ b/python/ql/lib/semmle/python/frameworks/Django.qll @@ -2965,38 +2965,6 @@ module PrivateDjango { override predicate csrfEnabled() { decoratorName in ["csrf_protect", "requires_csrf_token"] } } - private predicate djangoUrlHasAllowedHostAndScheme( - DataFlow::GuardNode g, ControlFlowNode node, boolean branch - ) { - exists(API::CallNode call | - call = - API::moduleImport("django") - .getMember("utils") - .getMember("http") - .getMember("url_has_allowed_host_and_scheme") - .getACall() and - g = call.asCfgNode() and - node = call.getParameter(0, "url").asSink().asCfgNode() and - branch = true - ) - } - - /** - * A call to `django.utils.http.url_has_allowed_host_and_scheme`, considered as a sanitizer-guard for URL redirection. - * - * See https://docs.djangoproject.com/en/4.2/_modules/django/utils/http/ - */ - private class DjangoAllowedUrl extends UrlRedirect::Sanitizer { - DjangoAllowedUrl() { - this = DataFlow::BarrierGuard::getABarrierNode() - } - - override predicate sanitizes(UrlRedirect::FlowState state) { - // sanitize all flow states - any() - } - } - // --------------------------------------------------------------------------- // Templates // --------------------------------------------------------------------------- diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll index 80ec45a3cf17..ee2cd8f57bfe 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll @@ -344,6 +344,26 @@ private predicate sinkModel(string type, string path, string kind, string model) ) } +/** Holds if a barrier model exists for the given parameters. */ +private predicate barrierModel(string type, string path, string kind, string model) { + // No deprecation adapter for barrier models, they were not around back then. + exists(QlBuiltins::ExtensionId madId | + Extensions::barrierModel(type, path, kind, madId) and + model = "MaD:" + madId.toString() + ) +} + +/** Holds if a barrier guard model exists for the given parameters. */ +private predicate barrierGuardModel( + string type, string path, string branch, string kind, string model +) { + // No deprecation adapter for barrier models, they were not around back then. + exists(QlBuiltins::ExtensionId madId | + Extensions::barrierGuardModel(type, path, branch, kind, madId) and + model = "MaD:" + madId.toString() + ) +} + /** Holds if a summary model `row` exists for the given parameters. */ private predicate summaryModel( string type, string path, string input, string output, string kind, string model @@ -400,6 +420,8 @@ predicate isRelevantType(string type) { ( sourceModel(type, _, _, _) or sinkModel(type, _, _, _) or + barrierModel(type, _, _, _) or + barrierGuardModel(type, _, _, _, _) or summaryModel(type, _, _, _, _, _) or typeModel(_, type, _) ) and @@ -427,6 +449,8 @@ predicate isRelevantFullPath(string type, string path) { ( sourceModel(type, path, _, _) or sinkModel(type, path, _, _) or + barrierModel(type, path, _, _) or + barrierGuardModel(type, path, _, _, _) or summaryModel(type, path, _, _, _, _) or typeModel(_, type, path) ) @@ -745,6 +769,32 @@ module ModelOutput { ) } + /** + * Holds if a barrier model contributed `barrier` with the given `kind`. + */ + cached + API::Node getABarrierNode(string kind, string model) { + exists(string type, string path | + barrierModel(type, path, kind, model) and + result = getNodeFromPath(type, path) + ) + } + + /** + * Holds if a barrier model contributed `barrier` with the given `kind`. + */ + cached + API::Node getABarrierGuardNode(string kind, boolean branch, string model) { + exists(string type, string path, string branch_str | + branch = true and branch_str = "true" + or + branch = false and branch_str = "false" + | + barrierGuardModel(type, path, branch_str, kind, model) and + result = getNodeFromPath(type, path) + ) + } + /** * Holds if a relevant summary exists for these parameters. */ @@ -787,15 +837,27 @@ module ModelOutput { private import codeql.mad.ModelValidation as SharedModelVal /** - * Holds if a CSV source model contributed `source` with the given `kind`. + * Holds if an external model contributed `source` with the given `kind`. */ API::Node getASourceNode(string kind) { result = getASourceNode(kind, _) } /** - * Holds if a CSV sink model contributed `sink` with the given `kind`. + * Holds if an external model contributed `sink` with the given `kind`. */ API::Node getASinkNode(string kind) { result = getASinkNode(kind, _) } + /** + * Holds if an external model contributed `barrier` with the given `kind`. + */ + API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) } + + /** + * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`. + */ + API::Node getABarrierGuardNode(string kind, boolean branch) { + result = getABarrierGuardNode(kind, branch, _) + } + private module KindValConfig implements SharedModelVal::KindValidationConfigSig { predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind, _) } diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll index 3f38c498f324..2a644aabb95d 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -20,6 +20,26 @@ extensible predicate sourceModel( */ extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId); +/** + * Holds if the value at `(type, path)` should be seen as a barrier + * of the given `kind` and `madId` is the data extension row number. + */ +extensible predicate barrierModel( + string type, string path, string kind, QlBuiltins::ExtensionId madId +); + +/** + * Holds if the value at `(type, path)` should be seen as a barrier guard + * of the given `kind` and `madId` is the data extension row number. + * `path` is assumed to lead to a parameter of a call (possibly `self`), and + * the call is guarding the parameter. + * `branch` is either `true` or `false`, indicating which branch of the guard + * is protecting the parameter. + */ +extensible predicate barrierGuardModel( + string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId +); + /** * Holds if in calls to `(type, path)`, the value referred to by `input` * can flow to the value referred to by `output` and `madId` is the data diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/empty.model.yml b/python/ql/lib/semmle/python/frameworks/data/internal/empty.model.yml index ea9b9fce546b..a7529031c29f 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/empty.model.yml +++ b/python/ql/lib/semmle/python/frameworks/data/internal/empty.model.yml @@ -11,6 +11,16 @@ extensions: extensible: sinkModel data: [] + - addsTo: + pack: codeql/python-all + extensible: barrierModel + data: [] + + - addsTo: + pack: codeql/python-all + extensible: barrierGuardModel + data: [] + - addsTo: pack: codeql/python-all extensible: summaryModel diff --git a/python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll index f5810944f8d9..a1c652192f41 100644 --- a/python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll +++ b/python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll @@ -7,6 +7,7 @@ private import python private import semmle.python.dataflow.new.DataFlow private import semmle.python.Concepts +private import semmle.python.ApiGraphs private import semmle.python.dataflow.new.RemoteFlowSources private import semmle.python.dataflow.new.BarrierGuards private import semmle.python.frameworks.data.ModelsAsData @@ -95,6 +96,9 @@ module UrlRedirect { } } + /** + * A sink for URL redirection defined via models-as-data. + */ private class SinkFromModel extends Sink { SinkFromModel() { this = ModelOutput::getASinkNode("url-redirection").asSink() } } @@ -156,4 +160,15 @@ module UrlRedirect { /** DEPRECATED: Use ConstCompareAsSanitizerGuard instead. */ deprecated class StringConstCompareAsSanitizerGuard = ConstCompareAsSanitizerGuard; + + class SanitizerFromModel extends Sanitizer { + SanitizerFromModel() { + this = DataFlow::ExternalBarrierGuard::getAnExternalBarrierNode("url-redirection") + } + + override predicate sanitizes(FlowState state) { + // sanitize all flow states + any() + } + } } diff --git a/python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected b/python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected index 0f334d45ea83..69bb8d30e8f4 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected +++ b/python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected @@ -75,7 +75,7 @@ edges | UnsafeUnpack.py:161:19:161:21 | ControlFlowNode for tar | UnsafeUnpack.py:163:33:163:35 | ControlFlowNode for tar | provenance | | | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | UnsafeUnpack.py:161:19:161:21 | ControlFlowNode for tar | provenance | | | UnsafeUnpack.py:161:38:161:45 | ControlFlowNode for savepath | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | provenance | Config | -| UnsafeUnpack.py:161:38:161:45 | ControlFlowNode for savepath | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | provenance | MaD:69 | +| UnsafeUnpack.py:161:38:161:45 | ControlFlowNode for savepath | UnsafeUnpack.py:161:25:161:46 | ControlFlowNode for Attribute() | provenance | MaD:70 | | UnsafeUnpack.py:163:23:163:28 | ControlFlowNode for member | UnsafeUnpack.py:166:37:166:42 | ControlFlowNode for member | provenance | | | UnsafeUnpack.py:163:33:163:35 | ControlFlowNode for tar | UnsafeUnpack.py:163:23:163:28 | ControlFlowNode for member | provenance | | | UnsafeUnpack.py:166:23:166:28 | [post] ControlFlowNode for result | UnsafeUnpack.py:167:67:167:72 | ControlFlowNode for result | provenance | | diff --git a/python/ql/test/experimental/query-tests/Security/CWE-409/DecompressionBombs.expected b/python/ql/test/experimental/query-tests/Security/CWE-409/DecompressionBombs.expected index 87b07df086fa..da68554f61db 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-409/DecompressionBombs.expected +++ b/python/ql/test/experimental/query-tests/Security/CWE-409/DecompressionBombs.expected @@ -1,23 +1,23 @@ edges | test.py:10:16:10:24 | ControlFlowNode for file_path | test.py:11:21:11:29 | ControlFlowNode for file_path | provenance | | | test.py:11:5:11:35 | ControlFlowNode for Attribute() | test.py:11:5:11:52 | ControlFlowNode for Attribute() | provenance | Config | -| test.py:11:21:11:29 | ControlFlowNode for file_path | test.py:11:5:11:35 | ControlFlowNode for Attribute() | provenance | MaD:86 | +| test.py:11:21:11:29 | ControlFlowNode for file_path | test.py:11:5:11:35 | ControlFlowNode for Attribute() | provenance | MaD:87 | | test.py:11:21:11:29 | ControlFlowNode for file_path | test.py:11:5:11:52 | ControlFlowNode for Attribute() | provenance | Config | | test.py:11:21:11:29 | ControlFlowNode for file_path | test.py:12:21:12:29 | ControlFlowNode for file_path | provenance | | | test.py:12:5:12:35 | ControlFlowNode for Attribute() | test.py:12:5:12:48 | ControlFlowNode for Attribute() | provenance | Config | -| test.py:12:21:12:29 | ControlFlowNode for file_path | test.py:12:5:12:35 | ControlFlowNode for Attribute() | provenance | MaD:86 | +| test.py:12:21:12:29 | ControlFlowNode for file_path | test.py:12:5:12:35 | ControlFlowNode for Attribute() | provenance | MaD:87 | | test.py:12:21:12:29 | ControlFlowNode for file_path | test.py:12:5:12:48 | ControlFlowNode for Attribute() | provenance | Config | | test.py:12:21:12:29 | ControlFlowNode for file_path | test.py:14:26:14:34 | ControlFlowNode for file_path | provenance | | | test.py:14:10:14:35 | ControlFlowNode for Attribute() | test.py:15:14:15:29 | ControlFlowNode for Attribute() | provenance | Config | -| test.py:14:26:14:34 | ControlFlowNode for file_path | test.py:14:10:14:35 | ControlFlowNode for Attribute() | provenance | MaD:86 | +| test.py:14:26:14:34 | ControlFlowNode for file_path | test.py:14:10:14:35 | ControlFlowNode for Attribute() | provenance | MaD:87 | | test.py:14:26:14:34 | ControlFlowNode for file_path | test.py:15:14:15:29 | ControlFlowNode for Attribute() | provenance | Config | | test.py:14:26:14:34 | ControlFlowNode for file_path | test.py:18:26:18:34 | ControlFlowNode for file_path | provenance | | | test.py:18:10:18:35 | ControlFlowNode for Attribute() | test.py:19:14:19:39 | ControlFlowNode for Attribute() | provenance | Config | -| test.py:18:26:18:34 | ControlFlowNode for file_path | test.py:18:10:18:35 | ControlFlowNode for Attribute() | provenance | MaD:86 | +| test.py:18:26:18:34 | ControlFlowNode for file_path | test.py:18:10:18:35 | ControlFlowNode for Attribute() | provenance | MaD:87 | | test.py:18:26:18:34 | ControlFlowNode for file_path | test.py:19:14:19:39 | ControlFlowNode for Attribute() | provenance | Config | | test.py:18:26:18:34 | ControlFlowNode for file_path | test.py:22:21:22:29 | ControlFlowNode for file_path | provenance | | | test.py:22:5:22:30 | ControlFlowNode for Attribute() | test.py:22:5:22:60 | ControlFlowNode for Attribute() | provenance | Config | -| test.py:22:21:22:29 | ControlFlowNode for file_path | test.py:22:5:22:30 | ControlFlowNode for Attribute() | provenance | MaD:86 | +| test.py:22:21:22:29 | ControlFlowNode for file_path | test.py:22:5:22:30 | ControlFlowNode for Attribute() | provenance | MaD:87 | | test.py:22:21:22:29 | ControlFlowNode for file_path | test.py:22:5:22:60 | ControlFlowNode for Attribute() | provenance | Config | | test.py:22:21:22:29 | ControlFlowNode for file_path | test.py:24:18:24:26 | ControlFlowNode for file_path | provenance | | | test.py:24:18:24:26 | ControlFlowNode for file_path | test.py:24:5:24:52 | ControlFlowNode for Attribute() | provenance | Config | diff --git a/python/ql/test/query-tests/Security/CWE-089-SqlInjection-local-threat-model/SqlInjection.expected b/python/ql/test/query-tests/Security/CWE-089-SqlInjection-local-threat-model/SqlInjection.expected index 1e4ba8b95305..d59e639d641b 100644 --- a/python/ql/test/query-tests/Security/CWE-089-SqlInjection-local-threat-model/SqlInjection.expected +++ b/python/ql/test/query-tests/Security/CWE-089-SqlInjection-local-threat-model/SqlInjection.expected @@ -1,5 +1,5 @@ edges -| test.py:6:14:6:21 | ControlFlowNode for Attribute | test.py:6:14:6:24 | ControlFlowNode for Subscript | provenance | Src:MaD:17 | +| test.py:6:14:6:21 | ControlFlowNode for Attribute | test.py:6:14:6:24 | ControlFlowNode for Subscript | provenance | Src:MaD:18 | nodes | test.py:6:14:6:21 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | | test.py:6:14:6:24 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript | diff --git a/python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests1/HeaderInjection.expected b/python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests1/HeaderInjection.expected index cea505fe39db..6c5f8363c487 100644 --- a/python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests1/HeaderInjection.expected +++ b/python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests1/HeaderInjection.expected @@ -14,10 +14,10 @@ edges | http_test.py:5:16:5:19 | ControlFlowNode for self | http_test.py:6:45:6:53 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | | http_test.py:6:9:6:19 | ControlFlowNode for parsed_path | http_test.py:7:40:7:56 | ControlFlowNode for Attribute | provenance | AdditionalTaintStep | | http_test.py:6:23:6:54 | ControlFlowNode for Attribute() | http_test.py:6:9:6:19 | ControlFlowNode for parsed_path | provenance | | -| http_test.py:6:45:6:53 | ControlFlowNode for Attribute | http_test.py:6:23:6:54 | ControlFlowNode for Attribute() | provenance | MaD:77 | +| http_test.py:6:45:6:53 | ControlFlowNode for Attribute | http_test.py:6:23:6:54 | ControlFlowNode for Attribute() | provenance | MaD:78 | | http_test.py:7:9:7:14 | ControlFlowNode for params | http_test.py:8:23:8:28 | ControlFlowNode for params | provenance | | | http_test.py:7:18:7:57 | ControlFlowNode for Attribute() | http_test.py:7:9:7:14 | ControlFlowNode for params | provenance | | -| http_test.py:7:40:7:56 | ControlFlowNode for Attribute | http_test.py:7:18:7:57 | ControlFlowNode for Attribute() | provenance | MaD:76 | +| http_test.py:7:40:7:56 | ControlFlowNode for Attribute | http_test.py:7:18:7:57 | ControlFlowNode for Attribute() | provenance | MaD:77 | | http_test.py:8:9:8:19 | ControlFlowNode for input_value | http_test.py:12:40:12:50 | ControlFlowNode for input_value | provenance | | | http_test.py:8:23:8:28 | ControlFlowNode for params | http_test.py:8:23:8:47 | ControlFlowNode for Attribute() | provenance | dict.get | | http_test.py:8:23:8:47 | ControlFlowNode for Attribute() | http_test.py:8:9:8:19 | ControlFlowNode for input_value | provenance | | diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll index 80ec45a3cf17..ee2cd8f57bfe 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll @@ -344,6 +344,26 @@ private predicate sinkModel(string type, string path, string kind, string model) ) } +/** Holds if a barrier model exists for the given parameters. */ +private predicate barrierModel(string type, string path, string kind, string model) { + // No deprecation adapter for barrier models, they were not around back then. + exists(QlBuiltins::ExtensionId madId | + Extensions::barrierModel(type, path, kind, madId) and + model = "MaD:" + madId.toString() + ) +} + +/** Holds if a barrier guard model exists for the given parameters. */ +private predicate barrierGuardModel( + string type, string path, string branch, string kind, string model +) { + // No deprecation adapter for barrier models, they were not around back then. + exists(QlBuiltins::ExtensionId madId | + Extensions::barrierGuardModel(type, path, branch, kind, madId) and + model = "MaD:" + madId.toString() + ) +} + /** Holds if a summary model `row` exists for the given parameters. */ private predicate summaryModel( string type, string path, string input, string output, string kind, string model @@ -400,6 +420,8 @@ predicate isRelevantType(string type) { ( sourceModel(type, _, _, _) or sinkModel(type, _, _, _) or + barrierModel(type, _, _, _) or + barrierGuardModel(type, _, _, _, _) or summaryModel(type, _, _, _, _, _) or typeModel(_, type, _) ) and @@ -427,6 +449,8 @@ predicate isRelevantFullPath(string type, string path) { ( sourceModel(type, path, _, _) or sinkModel(type, path, _, _) or + barrierModel(type, path, _, _) or + barrierGuardModel(type, path, _, _, _) or summaryModel(type, path, _, _, _, _) or typeModel(_, type, path) ) @@ -745,6 +769,32 @@ module ModelOutput { ) } + /** + * Holds if a barrier model contributed `barrier` with the given `kind`. + */ + cached + API::Node getABarrierNode(string kind, string model) { + exists(string type, string path | + barrierModel(type, path, kind, model) and + result = getNodeFromPath(type, path) + ) + } + + /** + * Holds if a barrier model contributed `barrier` with the given `kind`. + */ + cached + API::Node getABarrierGuardNode(string kind, boolean branch, string model) { + exists(string type, string path, string branch_str | + branch = true and branch_str = "true" + or + branch = false and branch_str = "false" + | + barrierGuardModel(type, path, branch_str, kind, model) and + result = getNodeFromPath(type, path) + ) + } + /** * Holds if a relevant summary exists for these parameters. */ @@ -787,15 +837,27 @@ module ModelOutput { private import codeql.mad.ModelValidation as SharedModelVal /** - * Holds if a CSV source model contributed `source` with the given `kind`. + * Holds if an external model contributed `source` with the given `kind`. */ API::Node getASourceNode(string kind) { result = getASourceNode(kind, _) } /** - * Holds if a CSV sink model contributed `sink` with the given `kind`. + * Holds if an external model contributed `sink` with the given `kind`. */ API::Node getASinkNode(string kind) { result = getASinkNode(kind, _) } + /** + * Holds if an external model contributed `barrier` with the given `kind`. + */ + API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) } + + /** + * Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`. + */ + API::Node getABarrierGuardNode(string kind, boolean branch) { + result = getABarrierGuardNode(kind, branch, _) + } + private module KindValConfig implements SharedModelVal::KindValidationConfigSig { predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind, _) } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll index 3f38c498f324..2a644aabb95d 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsExtensions.qll @@ -20,6 +20,26 @@ extensible predicate sourceModel( */ extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId); +/** + * Holds if the value at `(type, path)` should be seen as a barrier + * of the given `kind` and `madId` is the data extension row number. + */ +extensible predicate barrierModel( + string type, string path, string kind, QlBuiltins::ExtensionId madId +); + +/** + * Holds if the value at `(type, path)` should be seen as a barrier guard + * of the given `kind` and `madId` is the data extension row number. + * `path` is assumed to lead to a parameter of a call (possibly `self`), and + * the call is guarding the parameter. + * `branch` is either `true` or `false`, indicating which branch of the guard + * is protecting the parameter. + */ +extensible predicate barrierGuardModel( + string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId +); + /** * Holds if in calls to `(type, path)`, the value referred to by `input` * can flow to the value referred to by `output` and `madId` is the data diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/empty.model.yml b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/empty.model.yml index b887eed7c1c5..ec68e8fcb380 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/empty.model.yml +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/empty.model.yml @@ -16,6 +16,16 @@ extensions: extensible: summaryModel data: [] + - addsTo: + pack: codeql/ruby-all + extensible: barrierModel + data: [] + + - addsTo: + pack: codeql/ruby-all + extensible: barrierGuardModel + data: [] + - addsTo: pack: codeql/ruby-all extensible: neutralModel diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index 6cc9d6f88a45..4ab2eb1650c6 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -215,6 +215,35 @@ module Make< ] } + class AcceptingValue extends string { + AcceptingValue() { + this = + [ + "true", + "false", + "no-exception", + "zero", + "not-zero", + "null", + "not-null", + ] + } + + predicate isTrue() { this = "true" } + + predicate isFalse() { this = "false" } + + predicate isNoException() { this = "no-exception" } + + predicate isZero() { this = "zero" } + + predicate isNotZero() { this = "not-zero" } + + predicate isNull() { this = "null" } + + predicate isNotNull() { this = "not-null" } + } + /** * A class used to represent provenance values for MaD models. * @@ -2015,6 +2044,12 @@ module Make< not exists(interpretComponent(c)) } + /** Holds if `acceptingvalue` is not a valid barrier guard accepting-value. */ + bindingset[acceptingvalue] + predicate invalidAcceptingValue(string acceptingvalue) { + not acceptingvalue instanceof AcceptingValue + } + /** Holds if `provenance` is not a valid provenance value. */ bindingset[provenance] predicate invalidProvenance(string provenance) { not provenance instanceof Provenance } @@ -2052,6 +2087,23 @@ module Make< Element n, string input, string kind, Provenance provenance, string model ); + /** + * Holds if an external barrier specification exists for `n` with output specification + * `output` and kind `kind`. + */ + predicate barrierElement( + Element n, string output, string kind, Provenance provenance, string model + ); + + /** + * Holds if an external barrier guard specification exists for `n` with input + * specification `input`, accepting value `acceptingvalue`, and kind `kind`. + */ + predicate barrierGuardElement( + Element n, string input, AcceptingValue acceptingvalue, string kind, + Provenance provenance, string model + ); + class SourceOrSinkElement extends Element; /** An entity used to interpret a source/sink specification. */ @@ -2105,7 +2157,9 @@ module Make< private predicate sourceSinkSpec(string spec) { sourceElement(_, spec, _, _, _) or - sinkElement(_, spec, _, _, _) + sinkElement(_, spec, _, _, _) or + barrierElement(_, spec, _, _, _) or + barrierGuardElement(_, spec, _, _, _, _) } private module AccessPath = AccessPathSyntax::AccessPath; @@ -2160,11 +2214,34 @@ module Make< ) } + private predicate barrierElementRef( + InterpretNode ref, SourceSinkAccessPath output, string kind, string model + ) { + exists(SourceOrSinkElement e | + barrierElement(e, output, kind, _, model) and + if outputNeedsReferenceExt(output.getToken(0)) + then e = ref.getCallTarget() + else e = ref.asElement() + ) + } + + private predicate barrierGuardElementRef( + InterpretNode ref, SourceSinkAccessPath input, AcceptingValue acceptingvalue, string kind, + string model + ) { + exists(SourceOrSinkElement e | + barrierGuardElement(e, input, acceptingvalue, kind, _, model) and + if inputNeedsReferenceExt(input.getToken(0)) + then e = ref.getCallTarget() + else e = ref.asElement() + ) + } + /** Holds if the first `n` tokens of `output` resolve to the given interpretation. */ private predicate interpretOutput( SourceSinkAccessPath output, int n, InterpretNode ref, InterpretNode node ) { - sourceElementRef(ref, output, _, _) and + (sourceElementRef(ref, output, _, _) or barrierElementRef(ref, output, _, _)) and n = 0 and ( if output = "" @@ -2220,7 +2297,7 @@ module Make< private predicate interpretInput( SourceSinkAccessPath input, int n, InterpretNode ref, InterpretNode node ) { - sinkElementRef(ref, input, _, _) and + (sinkElementRef(ref, input, _, _) or barrierGuardElementRef(ref, input, _, _, _)) and n = 0 and ( if input = "" @@ -2280,6 +2357,30 @@ module Make< ) } + /** + * Holds if `node` is specified as a barrier with the given kind in a MaD flow + * model. + */ + predicate isBarrierNode(InterpretNode node, string kind, string model) { + exists(InterpretNode ref, SourceSinkAccessPath output | + barrierElementRef(ref, output, kind, model) and + interpretOutput(output, output.getNumToken(), ref, node) + ) + } + + /** + * Holds if `node` is specified as a barrier guard argument with the + * given kind in a MaD flow model. + */ + predicate isBarrierGuardNode( + InterpretNode node, AcceptingValue acceptingvalue, string kind, string model + ) { + exists(InterpretNode ref, SourceSinkAccessPath input | + barrierGuardElementRef(ref, input, acceptingvalue, kind, model) and + interpretInput(input, input.getNumToken(), ref, node) + ) + } + final private class SourceOrSinkElementFinal = SourceOrSinkElement; signature predicate sourceOrSinkElementSig( diff --git a/shared/mad/codeql/mad/ModelValidation.qll b/shared/mad/codeql/mad/ModelValidation.qll index 5d4698bed1d4..9791355d03ae 100644 --- a/shared/mad/codeql/mad/ModelValidation.qll +++ b/shared/mad/codeql/mad/ModelValidation.qll @@ -173,7 +173,7 @@ module KindValidation { or exists(string kind, string msg | Config::sinkKind(kind) | not kind instanceof ValidSinkKind and - msg = "Invalid kind \"" + kind + "\" in sink model." and + msg = "Invalid kind \"" + kind + "\" in sink or barrier model." and // The part of this message that refers to outdated sink kinds can be deleted after June 1st, 2024. if kind instanceof OutdatedSinkKind then result = msg + " " + kind.(OutdatedSinkKind).outdatedMessage() diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll b/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll index 46e2e63f2cab..692e3626080d 100644 --- a/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll +++ b/swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll @@ -159,6 +159,19 @@ module SourceSinkInterpretationInput implements ) } + predicate barrierElement( + Element n, string output, string kind, Public::Provenance provenance, string model + ) { + none() + } + + predicate barrierGuardElement( + Element n, string input, Public::AcceptingValue acceptingvalue, string kind, + Public::Provenance provenance, string model + ) { + none() + } + private newtype TInterpretNode = TElement_(Element n) or TNode_(Node n) or