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..c712e1ae5fb6 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 input, 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 originalInput, QlBuiltins::ExtensionId madId + | + barrierGuardModel(namespace, type, subtypes, name, signature, ext, originalInput, + acceptingvalue, kind, provenance, madId) and + model = "MaD:" + madId.toString() and + baseBarrier = interpretElement(namespace, type, subtypes, name, signature, ext, _) and + ( + e = baseBarrier and input = originalInput + or + correspondingKotlinParameterDefaultsArgSpec(baseBarrier, e, originalInput, input) + ) + ) + } + 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/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