From 9125fe6f05c0526002b216c523ee881322d6ff9f Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 11 Dec 2025 14:23:21 +0100 Subject: [PATCH 01/11] Go: Add support for MaD barriers and barrier guards. --- go/ql/lib/semmle/go/dataflow/ExternalFlow.qll | 58 +++++++++++++++ .../go/dataflow/internal/DataFlowUtil.qll | 70 +++++++++++++++---- .../go/dataflow/internal/FlowSummaryImpl.qll | 19 +++-- 3 files changed, 128 insertions(+), 19 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll index de1e3da62813..cea1a00e3e8a 100644 --- a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll +++ b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll @@ -262,6 +262,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 = false @@ -397,6 +401,54 @@ private module Cached { isSinkNode(n, kind, model) and n.asNode() = node ) } + + private newtype TKindModelPair = + TMkPair(string kind, string model) { isBarrierGuardNode(_, _, kind, model) } + + private boolean convertAcceptingValue(Public::AcceptingValue av) { + av.isTrue() and result = true + or + av.isFalse() and result = false + // Remaining cases are not supported yet, they depend on the shared Guards library. + // 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(DataFlow::Node g, Expr e, boolean gv, TKindModelPair kmp) { + exists( + SourceSinkInterpretationInput::InterpretNode n, Public::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.asExpr().(CallExpr).getAnArgument() = e // TODO: qualifier? + ) + } + + /** + * Holds if `node` is specified as a barrier with the given kind in a MaD flow + * model. + */ + cached + predicate barrierNode(DataFlow::Node node, string kind, string model) { + exists(SourceSinkInterpretationInput::InterpretNode n | + isBarrierNode(n, kind, model) and n.asNode() = node + ) + or + DataFlow::ParameterizedBarrierGuard::getABarrierNode(TMkPair(kind, + model)) = node + } } import Cached @@ -413,6 +465,12 @@ predicate sourceNode(DataFlow::Node node, string kind) { sourceNode(node, kind, */ predicate sinkNode(DataFlow::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(DataFlow::Node node, string kind) { barrierNode(node, kind, _) } + // adapter class for converting Mad summaries to `SummarizedCallable`s private class SummarizedCallableAdapter extends Public::SummarizedCallable { SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _) } diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll index 14ff455646c9..404eca4b4a25 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -339,6 +339,20 @@ class ContentSet instanceof TContentSet { */ signature predicate guardChecksSig(Node g, Expr e, boolean branch); +bindingset[this] +private signature class ParamSig; + +private module WithParam { + /** + * Holds if the guard `g` validates the expression `e` upon evaluating to `branch`. + * + * 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(Node g, Expr e, boolean branch, P param); +} + /** * Provides a set of barrier nodes for a guard that validates an expression. * @@ -346,12 +360,36 @@ signature predicate guardChecksSig(Node g, Expr e, boolean branch); * in data flow and taint tracking. */ module BarrierGuard { + private predicate guardChecks(Node g, Expr e, boolean branch, Unit param) { + guardChecks(g, e, branch) and exists(param) + } + + private module B = ParameterizedBarrierGuard; + + /** Gets a node that is safely guarded by the given guard check. */ + Node getABarrierNode() { result = B::getABarrierNode(_) } + + /** + * Gets a node that is safely guarded by the given guard check. + */ + Node getABarrierNodeForGuard(Node guardCheck) { + result = B::getABarrierNodeForGuard(guardCheck, _) + } +} + +/** + * 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() { + Node getABarrierNode(P param) { exists(ControlFlow::ConditionGuardNode guard, SsaWithFields var | result = pragma[only_bind_out](var).getAUse() | - guards(_, guard, _, var) and + guards(_, guard, _, var, param) and pragma[only_bind_out](guard).dominates(result.getBasicBlock()) ) } @@ -359,9 +397,9 @@ module BarrierGuard { /** * Gets a node that is safely guarded by the given guard check. */ - Node getABarrierNodeForGuard(Node guardCheck) { + Node getABarrierNodeForGuard(Node guardCheck, P param) { exists(ControlFlow::ConditionGuardNode guard, SsaWithFields var | result = var.getAUse() | - guards(guardCheck, guard, _, var) and + guards(guardCheck, guard, _, var, param) and guard.dominates(result.getBasicBlock()) ) } @@ -373,22 +411,24 @@ module BarrierGuard { * This predicate exists to enforce a good join order in `getAGuardedNode`. */ pragma[noinline] - private predicate guards(Node g, ControlFlow::ConditionGuardNode guard, Node nd, SsaWithFields ap) { - guards(g, guard, nd) and nd = ap.getAUse() + private predicate guards( + Node g, ControlFlow::ConditionGuardNode guard, Node nd, SsaWithFields ap, P param + ) { + guards(g, guard, nd, param) and nd = ap.getAUse() } /** * Holds if `guard` marks a point in the control-flow graph where `g` * is known to validate `nd`. */ - private predicate guards(Node g, ControlFlow::ConditionGuardNode guard, Node nd) { + private predicate guards(Node g, ControlFlow::ConditionGuardNode guard, Node nd, P param) { exists(boolean branch | - guardChecks(g, nd.asExpr(), branch) and + guardChecks(g, nd.asExpr(), branch, param) and guard.ensures(g, branch) ) or exists(DataFlow::Property p, Node resNode, Node check, boolean outcome | - guardingCall(g, _, _, _, p, _, nd, resNode) and + guardingCall(g, _, _, _, p, _, nd, resNode, param) and p.checkOn(check, outcome, resNode) and guard.ensures(pragma[only_bind_into](check), outcome) ) @@ -405,9 +445,9 @@ module BarrierGuard { pragma[noinline] private predicate guardingCall( Node g, Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p, CallNode c, - Node nd, Node resNode + Node nd, Node resNode, P param ) { - guardingFunction(g, f, inp, outp, p) and + guardingFunction(g, f, inp, outp, p, param) and c = f.getACall() and nd = getInputNode(inp, c) and localFlow(getOutputNode(outp, c), resNode) @@ -438,7 +478,7 @@ module BarrierGuard { * `false`, `nil` or a non-`nil` value.) */ private predicate guardingFunction( - Node g, Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p + Node g, Function f, FunctionInput inp, FunctionOutput outp, DataFlow::Property p, P param ) { exists(FuncDecl fd, Node arg, Node ret | fd.getFunction() = f and @@ -446,7 +486,7 @@ module BarrierGuard { ( // Case: a function like "if someBarrierGuard(arg) { return true } else { return false }" exists(ControlFlow::ConditionGuardNode guard | - guards(g, pragma[only_bind_out](guard), arg) and + guards(g, pragma[only_bind_out](guard), arg, param) and guard.dominates(pragma[only_bind_out](ret).getBasicBlock()) | onlyPossibleReturnSatisfyingProperty(fd, outp, ret, p) @@ -456,7 +496,7 @@ module BarrierGuard { // or "return !someBarrierGuard(arg) && otherCond(...)" exists(boolean outcome | ret = getUniqueOutputNode(fd, outp) and - guardChecks(g, arg.asExpr(), outcome) and + guardChecks(g, arg.asExpr(), outcome, param) and // This predicate's contract is (p holds of ret ==> arg is checked), // (and we have (this has outcome ==> arg is checked)) // but p.checkOn(ret, outcome, this) gives us (ret has outcome ==> p holds of this), @@ -471,7 +511,7 @@ module BarrierGuard { DataFlow::Property outpProp | ret = getUniqueOutputNode(fd, outp) and - guardingFunction(g, f2, inp2, outp2, outpProp) and + guardingFunction(g, f2, inp2, outp2, outpProp, param) and c = f2.getACall() and arg = inp2.getNode(c) and ( diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index 6c49cb75bd3e..633864fbf8c6 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -160,16 +160,27 @@ module SourceSinkInterpretationInput implements } predicate barrierElement( - Element n, string output, string kind, Public::Provenance provenance, string model + Element e, string output, string kind, Public::Provenance provenance, string model ) { - none() + exists( + string package, string type, boolean subtypes, string name, string signature, string ext + | + barrierModel(package, type, subtypes, name, signature, ext, output, kind, provenance, model) and + e = interpretElement(package, type, subtypes, name, signature, ext) + ) } predicate barrierGuardElement( - Element n, string input, Public::AcceptingValue acceptingvalue, string kind, + Element e, string input, Public::AcceptingValue acceptingvalue, string kind, Public::Provenance provenance, string model ) { - none() + exists( + string package, string type, boolean subtypes, string name, string signature, string ext + | + barrierGuardModel(package, type, subtypes, name, signature, ext, input, acceptingvalue, kind, + provenance, model) and + e = interpretElement(package, type, subtypes, name, signature, ext) + ) } // Note that due to embedding, which is currently implemented via some From a80ae3b8c716192ce50db5c9b417b8b7a3e40bcc Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 12:15:19 +0000 Subject: [PATCH 02/11] Add post-processing to Beego tests --- .../Beego/CleartextLogging.expected | 137 ++++++----- .../frameworks/Beego/CleartextLogging.qlref | 5 +- .../go/frameworks/Beego/OpenRedirect.expected | 6 +- .../go/frameworks/Beego/OpenRedirect.qlref | 5 +- .../go/frameworks/Beego/ReflectedXss.qlref | 4 +- .../go/frameworks/Beego/TaintedPath.qlref | 4 +- .../semmle/go/frameworks/Beego/test.go | 228 +++++++++--------- 7 files changed, 211 insertions(+), 178 deletions(-) diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.expected b/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.expected index 591d990be47c..30a38580f789 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.expected +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.expected @@ -1,3 +1,38 @@ +#select +| test.go:154:14:154:21 | password | test.go:153:17:153:24 | definition of password | test.go:154:14:154:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:155:17:155:24 | password | test.go:153:17:153:24 | definition of password | test.go:155:17:155:24 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:156:14:156:21 | password | test.go:153:17:153:24 | definition of password | test.go:156:14:156:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:157:18:157:25 | password | test.go:153:17:153:24 | definition of password | test.go:157:18:157:25 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:158:14:158:21 | password | test.go:153:17:153:24 | definition of password | test.go:158:14:158:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:159:13:159:20 | password | test.go:153:17:153:24 | definition of password | test.go:159:13:159:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:160:22:160:29 | password | test.go:153:17:153:24 | definition of password | test.go:160:22:160:29 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:161:15:161:22 | password | test.go:153:17:153:24 | definition of password | test.go:161:15:161:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:162:14:162:21 | password | test.go:153:17:153:24 | definition of password | test.go:162:14:162:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:163:13:163:20 | password | test.go:153:17:153:24 | definition of password | test.go:163:13:163:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:164:16:164:23 | password | test.go:153:17:153:24 | definition of password | test.go:164:16:164:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:165:13:165:20 | password | test.go:153:17:153:24 | definition of password | test.go:165:13:165:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:166:16:166:23 | password | test.go:153:17:153:24 | definition of password | test.go:166:16:166:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:167:13:167:20 | password | test.go:153:17:153:24 | definition of password | test.go:167:13:167:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:168:17:168:24 | password | test.go:153:17:153:24 | definition of password | test.go:168:17:168:24 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:169:13:169:20 | password | test.go:153:17:153:24 | definition of password | test.go:169:13:169:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:170:12:170:19 | password | test.go:153:17:153:24 | definition of password | test.go:170:12:170:19 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:171:21:171:28 | password | test.go:153:17:153:24 | definition of password | test.go:171:21:171:28 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:172:14:172:21 | password | test.go:153:17:153:24 | definition of password | test.go:172:14:172:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:173:13:173:20 | password | test.go:153:17:153:24 | definition of password | test.go:173:13:173:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:174:12:174:19 | password | test.go:153:17:153:24 | definition of password | test.go:174:12:174:19 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:175:15:175:22 | password | test.go:153:17:153:24 | definition of password | test.go:175:15:175:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:176:15:176:22 | password | test.go:153:17:153:24 | definition of password | test.go:176:15:176:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:177:18:177:25 | password | test.go:153:17:153:24 | definition of password | test.go:177:18:177:25 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:178:15:178:22 | password | test.go:153:17:153:24 | definition of password | test.go:178:15:178:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:179:19:179:26 | password | test.go:153:17:153:24 | definition of password | test.go:179:19:179:26 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:180:15:180:22 | password | test.go:153:17:153:24 | definition of password | test.go:180:15:180:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:181:14:181:21 | password | test.go:153:17:153:24 | definition of password | test.go:181:14:181:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:182:23:182:30 | password | test.go:153:17:153:24 | definition of password | test.go:182:23:182:30 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:183:16:183:23 | password | test.go:153:17:153:24 | definition of password | test.go:183:16:183:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:184:15:184:22 | password | test.go:153:17:153:24 | definition of password | test.go:184:15:184:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:185:14:185:21 | password | test.go:153:17:153:24 | definition of password | test.go:185:14:185:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:186:17:186:24 | password | test.go:153:17:153:24 | definition of password | test.go:186:17:186:24 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | +| test.go:187:16:187:23 | password | test.go:153:17:153:24 | definition of password | test.go:187:16:187:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | edges | test.go:153:17:153:24 | definition of password | test.go:154:14:154:21 | password | provenance | | | test.go:153:17:153:24 | definition of password | test.go:155:17:155:24 | password | provenance | | @@ -10,29 +45,52 @@ edges | test.go:153:17:153:24 | definition of password | test.go:162:14:162:21 | password | provenance | | | test.go:153:17:153:24 | definition of password | test.go:163:13:163:20 | password | provenance | | | test.go:153:17:153:24 | definition of password | test.go:164:16:164:23 | password | provenance | | -| test.go:153:17:153:24 | definition of password | test.go:165:13:165:20 | password | provenance | Sink:MaD:380 | -| test.go:153:17:153:24 | definition of password | test.go:166:16:166:23 | password | provenance | Sink:MaD:381 | -| test.go:153:17:153:24 | definition of password | test.go:167:13:167:20 | password | provenance | Sink:MaD:382 | -| test.go:153:17:153:24 | definition of password | test.go:168:17:168:24 | password | provenance | Sink:MaD:383 | -| test.go:153:17:153:24 | definition of password | test.go:169:13:169:20 | password | provenance | Sink:MaD:384 | -| test.go:153:17:153:24 | definition of password | test.go:170:12:170:19 | password | provenance | Sink:MaD:385 | -| test.go:153:17:153:24 | definition of password | test.go:171:21:171:28 | password | provenance | Sink:MaD:386 | -| test.go:153:17:153:24 | definition of password | test.go:172:14:172:21 | password | provenance | Sink:MaD:387 | -| test.go:153:17:153:24 | definition of password | test.go:173:13:173:20 | password | provenance | Sink:MaD:388 | -| test.go:153:17:153:24 | definition of password | test.go:174:12:174:19 | password | provenance | Sink:MaD:389 | -| test.go:153:17:153:24 | definition of password | test.go:175:15:175:22 | password | provenance | Sink:MaD:390 | -| test.go:153:17:153:24 | definition of password | test.go:176:15:176:22 | password | provenance | Sink:MaD:391 | -| test.go:153:17:153:24 | definition of password | test.go:177:18:177:25 | password | provenance | Sink:MaD:392 | -| test.go:153:17:153:24 | definition of password | test.go:178:15:178:22 | password | provenance | Sink:MaD:393 | -| test.go:153:17:153:24 | definition of password | test.go:179:19:179:26 | password | provenance | Sink:MaD:394 | -| test.go:153:17:153:24 | definition of password | test.go:180:15:180:22 | password | provenance | Sink:MaD:395 | -| test.go:153:17:153:24 | definition of password | test.go:181:14:181:21 | password | provenance | Sink:MaD:396 | -| test.go:153:17:153:24 | definition of password | test.go:182:23:182:30 | password | provenance | Sink:MaD:397 | -| test.go:153:17:153:24 | definition of password | test.go:183:16:183:23 | password | provenance | Sink:MaD:398 | -| test.go:153:17:153:24 | definition of password | test.go:184:15:184:22 | password | provenance | Sink:MaD:399 | -| test.go:153:17:153:24 | definition of password | test.go:185:14:185:21 | password | provenance | Sink:MaD:400 | -| test.go:153:17:153:24 | definition of password | test.go:186:17:186:24 | password | provenance | Sink:MaD:401 | +| test.go:153:17:153:24 | definition of password | test.go:165:13:165:20 | password | provenance | Sink:MaD:1 | +| test.go:153:17:153:24 | definition of password | test.go:166:16:166:23 | password | provenance | Sink:MaD:2 | +| test.go:153:17:153:24 | definition of password | test.go:167:13:167:20 | password | provenance | Sink:MaD:3 | +| test.go:153:17:153:24 | definition of password | test.go:168:17:168:24 | password | provenance | Sink:MaD:4 | +| test.go:153:17:153:24 | definition of password | test.go:169:13:169:20 | password | provenance | Sink:MaD:5 | +| test.go:153:17:153:24 | definition of password | test.go:170:12:170:19 | password | provenance | Sink:MaD:6 | +| test.go:153:17:153:24 | definition of password | test.go:171:21:171:28 | password | provenance | Sink:MaD:7 | +| test.go:153:17:153:24 | definition of password | test.go:172:14:172:21 | password | provenance | Sink:MaD:8 | +| test.go:153:17:153:24 | definition of password | test.go:173:13:173:20 | password | provenance | Sink:MaD:9 | +| test.go:153:17:153:24 | definition of password | test.go:174:12:174:19 | password | provenance | Sink:MaD:10 | +| test.go:153:17:153:24 | definition of password | test.go:175:15:175:22 | password | provenance | Sink:MaD:11 | +| test.go:153:17:153:24 | definition of password | test.go:176:15:176:22 | password | provenance | Sink:MaD:12 | +| test.go:153:17:153:24 | definition of password | test.go:177:18:177:25 | password | provenance | Sink:MaD:13 | +| test.go:153:17:153:24 | definition of password | test.go:178:15:178:22 | password | provenance | Sink:MaD:14 | +| test.go:153:17:153:24 | definition of password | test.go:179:19:179:26 | password | provenance | Sink:MaD:15 | +| test.go:153:17:153:24 | definition of password | test.go:180:15:180:22 | password | provenance | Sink:MaD:16 | +| test.go:153:17:153:24 | definition of password | test.go:181:14:181:21 | password | provenance | Sink:MaD:17 | +| test.go:153:17:153:24 | definition of password | test.go:182:23:182:30 | password | provenance | Sink:MaD:18 | +| test.go:153:17:153:24 | definition of password | test.go:183:16:183:23 | password | provenance | Sink:MaD:19 | +| test.go:153:17:153:24 | definition of password | test.go:184:15:184:22 | password | provenance | Sink:MaD:20 | +| test.go:153:17:153:24 | definition of password | test.go:185:14:185:21 | password | provenance | Sink:MaD:21 | +| test.go:153:17:153:24 | definition of password | test.go:186:17:186:24 | password | provenance | Sink:MaD:22 | | test.go:153:17:153:24 | definition of password | test.go:187:16:187:23 | password | provenance | | +models +| 1 | Sink: group:beego-logs; ; false; Alert; ; ; Argument[0..1]; log-injection; manual | +| 2 | Sink: group:beego-logs; ; false; Critical; ; ; Argument[0..1]; log-injection; manual | +| 3 | Sink: group:beego-logs; ; false; Debug; ; ; Argument[0..1]; log-injection; manual | +| 4 | Sink: group:beego-logs; ; false; Emergency; ; ; Argument[0..1]; log-injection; manual | +| 5 | Sink: group:beego-logs; ; false; Error; ; ; Argument[0..1]; log-injection; manual | +| 6 | Sink: group:beego-logs; ; false; Info; ; ; Argument[0..1]; log-injection; manual | +| 7 | Sink: group:beego-logs; ; false; Informational; ; ; Argument[0..1]; log-injection; manual | +| 8 | Sink: group:beego-logs; ; false; Notice; ; ; Argument[0..1]; log-injection; manual | +| 9 | Sink: group:beego-logs; ; false; Trace; ; ; Argument[0..1]; log-injection; manual | +| 10 | Sink: group:beego-logs; ; false; Warn; ; ; Argument[0..1]; log-injection; manual | +| 11 | Sink: group:beego-logs; ; false; Warning; ; ; Argument[0..1]; log-injection; manual | +| 12 | Sink: group:beego-logs; BeeLogger; true; Alert; ; ; Argument[0..1]; log-injection; manual | +| 13 | Sink: group:beego-logs; BeeLogger; true; Critical; ; ; Argument[0..1]; log-injection; manual | +| 14 | Sink: group:beego-logs; BeeLogger; true; Debug; ; ; Argument[0..1]; log-injection; manual | +| 15 | Sink: group:beego-logs; BeeLogger; true; Emergency; ; ; Argument[0..1]; log-injection; manual | +| 16 | Sink: group:beego-logs; BeeLogger; true; Error; ; ; Argument[0..1]; log-injection; manual | +| 17 | Sink: group:beego-logs; BeeLogger; true; Info; ; ; Argument[0..1]; log-injection; manual | +| 18 | Sink: group:beego-logs; BeeLogger; true; Informational; ; ; Argument[0..1]; log-injection; manual | +| 19 | Sink: group:beego-logs; BeeLogger; true; Notice; ; ; Argument[0..1]; log-injection; manual | +| 20 | Sink: group:beego-logs; BeeLogger; true; Trace; ; ; Argument[0..1]; log-injection; manual | +| 21 | Sink: group:beego-logs; BeeLogger; true; Warn; ; ; Argument[0..1]; log-injection; manual | +| 22 | Sink: group:beego-logs; BeeLogger; true; Warning; ; ; Argument[0..1]; log-injection; manual | nodes | test.go:153:17:153:24 | definition of password | semmle.label | definition of password | | test.go:154:14:154:21 | password | semmle.label | password | @@ -70,38 +128,3 @@ nodes | test.go:186:17:186:24 | password | semmle.label | password | | test.go:187:16:187:23 | password | semmle.label | password | subpaths -#select -| test.go:154:14:154:21 | password | test.go:153:17:153:24 | definition of password | test.go:154:14:154:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:155:17:155:24 | password | test.go:153:17:153:24 | definition of password | test.go:155:17:155:24 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:156:14:156:21 | password | test.go:153:17:153:24 | definition of password | test.go:156:14:156:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:157:18:157:25 | password | test.go:153:17:153:24 | definition of password | test.go:157:18:157:25 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:158:14:158:21 | password | test.go:153:17:153:24 | definition of password | test.go:158:14:158:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:159:13:159:20 | password | test.go:153:17:153:24 | definition of password | test.go:159:13:159:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:160:22:160:29 | password | test.go:153:17:153:24 | definition of password | test.go:160:22:160:29 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:161:15:161:22 | password | test.go:153:17:153:24 | definition of password | test.go:161:15:161:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:162:14:162:21 | password | test.go:153:17:153:24 | definition of password | test.go:162:14:162:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:163:13:163:20 | password | test.go:153:17:153:24 | definition of password | test.go:163:13:163:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:164:16:164:23 | password | test.go:153:17:153:24 | definition of password | test.go:164:16:164:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:165:13:165:20 | password | test.go:153:17:153:24 | definition of password | test.go:165:13:165:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:166:16:166:23 | password | test.go:153:17:153:24 | definition of password | test.go:166:16:166:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:167:13:167:20 | password | test.go:153:17:153:24 | definition of password | test.go:167:13:167:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:168:17:168:24 | password | test.go:153:17:153:24 | definition of password | test.go:168:17:168:24 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:169:13:169:20 | password | test.go:153:17:153:24 | definition of password | test.go:169:13:169:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:170:12:170:19 | password | test.go:153:17:153:24 | definition of password | test.go:170:12:170:19 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:171:21:171:28 | password | test.go:153:17:153:24 | definition of password | test.go:171:21:171:28 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:172:14:172:21 | password | test.go:153:17:153:24 | definition of password | test.go:172:14:172:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:173:13:173:20 | password | test.go:153:17:153:24 | definition of password | test.go:173:13:173:20 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:174:12:174:19 | password | test.go:153:17:153:24 | definition of password | test.go:174:12:174:19 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:175:15:175:22 | password | test.go:153:17:153:24 | definition of password | test.go:175:15:175:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:176:15:176:22 | password | test.go:153:17:153:24 | definition of password | test.go:176:15:176:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:177:18:177:25 | password | test.go:153:17:153:24 | definition of password | test.go:177:18:177:25 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:178:15:178:22 | password | test.go:153:17:153:24 | definition of password | test.go:178:15:178:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:179:19:179:26 | password | test.go:153:17:153:24 | definition of password | test.go:179:19:179:26 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:180:15:180:22 | password | test.go:153:17:153:24 | definition of password | test.go:180:15:180:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:181:14:181:21 | password | test.go:153:17:153:24 | definition of password | test.go:181:14:181:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:182:23:182:30 | password | test.go:153:17:153:24 | definition of password | test.go:182:23:182:30 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:183:16:183:23 | password | test.go:153:17:153:24 | definition of password | test.go:183:16:183:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:184:15:184:22 | password | test.go:153:17:153:24 | definition of password | test.go:184:15:184:22 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:185:14:185:21 | password | test.go:153:17:153:24 | definition of password | test.go:185:14:185:21 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:186:17:186:24 | password | test.go:153:17:153:24 | definition of password | test.go:186:17:186:24 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | -| test.go:187:16:187:23 | password | test.go:153:17:153:24 | definition of password | test.go:187:16:187:23 | password | $@ flows to a logging call. | test.go:153:17:153:24 | definition of password | Sensitive data returned by an access to password | diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.qlref b/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.qlref index 21eebbd09beb..693299c33a21 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.qlref +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/CleartextLogging.qlref @@ -1 +1,4 @@ -Security/CWE-312/CleartextLogging.ql +query: Security/CWE-312/CleartextLogging.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.expected b/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.expected index c624f05d450e..4dca9d050d71 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.expected +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.expected @@ -1,3 +1,6 @@ +#select +| test.go:253:13:253:34 | call to GetString | test.go:253:13:253:34 | call to GetString | test.go:253:13:253:34 | call to GetString | This path to an untrusted URL redirection depends on a $@. | test.go:253:13:253:34 | call to GetString | user-provided value | +| test.go:254:20:254:41 | call to GetString | test.go:254:20:254:41 | call to GetString | test.go:254:20:254:41 | call to GetString | This path to an untrusted URL redirection depends on a $@. | test.go:254:20:254:41 | call to GetString | user-provided value | edges nodes | test.go:253:13:253:34 | call to GetString | semmle.label | call to GetString | @@ -5,6 +8,3 @@ nodes | test.go:317:13:317:27 | call to URI | semmle.label | call to URI | | test.go:318:20:318:34 | call to URL | semmle.label | call to URL | subpaths -#select -| test.go:253:13:253:34 | call to GetString | test.go:253:13:253:34 | call to GetString | test.go:253:13:253:34 | call to GetString | This path to an untrusted URL redirection depends on a $@. | test.go:253:13:253:34 | call to GetString | user-provided value | -| test.go:254:20:254:41 | call to GetString | test.go:254:20:254:41 | call to GetString | test.go:254:20:254:41 | call to GetString | This path to an untrusted URL redirection depends on a $@. | test.go:254:20:254:41 | call to GetString | user-provided value | diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.qlref b/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.qlref index 0f1a74778256..13add930f517 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.qlref +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/OpenRedirect.qlref @@ -1 +1,4 @@ -Security/CWE-601/OpenUrlRedirect.ql +query: Security/CWE-601/OpenUrlRedirect.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/ReflectedXss.qlref b/go/ql/test/library-tests/semmle/go/frameworks/Beego/ReflectedXss.qlref index 754513d72bb3..e6b791f39fca 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/ReflectedXss.qlref +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/ReflectedXss.qlref @@ -1,2 +1,4 @@ query: Security/CWE-079/ReflectedXss.ql -postprocess: utils/test/PrettyPrintModels.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/TaintedPath.qlref b/go/ql/test/library-tests/semmle/go/frameworks/Beego/TaintedPath.qlref index 78ce25b1921f..6eb2e94892f2 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/TaintedPath.qlref +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/TaintedPath.qlref @@ -1,2 +1,4 @@ query: Security/CWE-022/TaintedPath.ql -postprocess: utils/test/PrettyPrintModels.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go b/go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go index d702f4a54450..38cb06691f83 100644 --- a/go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go +++ b/go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go @@ -31,75 +31,75 @@ type bindMe struct { // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromBind(input *context.BeegoInput, sink http.ResponseWriter) { var bound bindMe - input.Bind(bound, "someKey") - sink.Write([]byte(bound.a[0])) - sink.Write([]byte(bound.b)) - sink.Write([]byte(bound.c.z)) + input.Bind(bound, "someKey") // $ Source[go/reflected-xss] + sink.Write([]byte(bound.a[0])) // $ Alert[go/reflected-xss] + sink.Write([]byte(bound.b)) // $ Alert[go/reflected-xss] + sink.Write([]byte(bound.c.z)) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromCookie(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Cookie("someKey"))) + sink.Write([]byte(input.Cookie("someKey"))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromData(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Data()["someKey"].(string))) + sink.Write([]byte(input.Data()["someKey"].(string))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromGetData(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.GetData("someKey").(string))) + sink.Write([]byte(input.GetData("someKey").(string))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromHeader(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Header("someKey"))) + sink.Write([]byte(input.Header("someKey"))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromParam(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Param("someKey"))) + sink.Write([]byte(input.Param("someKey"))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromParams(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Params()["someKey"])) + sink.Write([]byte(input.Params()["someKey"])) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromQuery(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Query("someKey"))) + sink.Write([]byte(input.Query("someKey"))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromRefer(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Refer())) + sink.Write([]byte(input.Refer())) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromReferer(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.Referer())) + sink.Write([]byte(input.Referer())) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromURI(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.URI())) + sink.Write([]byte(input.URI())) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromURL(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.URL())) + sink.Write([]byte(input.URL())) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data to an `http.ResponseWriter` func xssFromUserAgent(input *context.BeegoInput, sink http.ResponseWriter) { - sink.Write([]byte(input.UserAgent())) + sink.Write([]byte(input.UserAgent())) // $ Alert[go/reflected-xss] } // BAD: with no obvious ContentType call we assume this could be text/html. func echoToBodyNoContentType(input *context.BeegoInput, output *context.BeegoOutput) { - output.Body(input.Data()["someKey"].([]byte)) + output.Body(input.Data()["someKey"].([]byte)) // $ Alert[go/reflected-xss] } // OK: JSON can't (by itself) cause XSS @@ -111,7 +111,7 @@ func echoToBodyContentTypeJson(input *context.BeegoInput, output *context.BeegoO // BAD: echoing untrusted data with an HTML content type func echoToBodyContentTypeHtml(input *context.BeegoInput, output *context.BeegoOutput) { output.ContentType("text/html") - output.Body(input.Data()["someKey"].([]byte)) + output.Body(input.Data()["someKey"].([]byte)) // $ Alert[go/reflected-xss] } // OK: JSON can't (by itself) cause XSS @@ -123,7 +123,7 @@ func echoToBodyContentTypeJsonUsingHeader(input *context.BeegoInput, output *con // BAD: echoing untrusted data with an HTML content type func echoToBodyContentTypeHtmlUsingHeader(input *context.BeegoInput, output *context.BeegoOutput) { output.Header("content-type", "text/html") - output.Body(input.Data()["someKey"].([]byte)) + output.Body(input.Data()["someKey"].([]byte)) // $ Alert[go/reflected-xss] } // OK: JSON and other non-HTML formats can't (by themselves) cause XSS @@ -140,7 +140,7 @@ func echoToFixedContentTypeRoutines(input *context.BeegoInput, output *context.B func echoToBodyContentTypeHtmlUsingHandler() { beego.Post("", func(context *context.Context) { context.Output.Header("content-type", "text/html") - context.Output.Body(context.Input.Data()["someKey"].([]byte)) + context.Output.Body(context.Input.Data()["someKey"].([]byte)) // $ Alert[go/reflected-xss] }) } @@ -150,41 +150,41 @@ func echoToBodySanitized(input *context.BeegoInput, output *context.BeegoOutput) } // BAD: logging something named "password". -func loggerTest(password string, logger *logs.BeeLogger) { - beego.Alert(password) - beego.Critical(password) - beego.Debug(password) - beego.Emergency(password) - beego.Error(password) - beego.Info(password) - beego.Informational(password) - beego.Notice(password) - beego.Trace(password) - beego.Warn(password) - beego.Warning(password) - logs.Alert(password) - logs.Critical(password) - logs.Debug(password) - logs.Emergency(password) - logs.Error(password) - logs.Info(password) - logs.Informational(password) - logs.Notice(password) - logs.Trace(password) - logs.Warn(password) - logs.Warning(password) - logger.Alert(password) - logger.Critical(password) - logger.Debug(password) - logger.Emergency(password) - logger.Error(password) - logger.Info(password) - logger.Informational(password) - logger.Notice(password) - logger.Trace(password) - logger.Warn(password) - logger.Warning(password) - utils.Display(password) +func loggerTest(password string, logger *logs.BeeLogger) { // $ Source[go/clear-text-logging] + beego.Alert(password) // $ Alert[go/clear-text-logging] + beego.Critical(password) // $ Alert[go/clear-text-logging] + beego.Debug(password) // $ Alert[go/clear-text-logging] + beego.Emergency(password) // $ Alert[go/clear-text-logging] + beego.Error(password) // $ Alert[go/clear-text-logging] + beego.Info(password) // $ Alert[go/clear-text-logging] + beego.Informational(password) // $ Alert[go/clear-text-logging] + beego.Notice(password) // $ Alert[go/clear-text-logging] + beego.Trace(password) // $ Alert[go/clear-text-logging] + beego.Warn(password) // $ Alert[go/clear-text-logging] + beego.Warning(password) // $ Alert[go/clear-text-logging] + logs.Alert(password) // $ Alert[go/clear-text-logging] + logs.Critical(password) // $ Alert[go/clear-text-logging] + logs.Debug(password) // $ Alert[go/clear-text-logging] + logs.Emergency(password) // $ Alert[go/clear-text-logging] + logs.Error(password) // $ Alert[go/clear-text-logging] + logs.Info(password) // $ Alert[go/clear-text-logging] + logs.Informational(password) // $ Alert[go/clear-text-logging] + logs.Notice(password) // $ Alert[go/clear-text-logging] + logs.Trace(password) // $ Alert[go/clear-text-logging] + logs.Warn(password) // $ Alert[go/clear-text-logging] + logs.Warning(password) // $ Alert[go/clear-text-logging] + logger.Alert(password) // $ Alert[go/clear-text-logging] + logger.Critical(password) // $ Alert[go/clear-text-logging] + logger.Debug(password) // $ Alert[go/clear-text-logging] + logger.Emergency(password) // $ Alert[go/clear-text-logging] + logger.Error(password) // $ Alert[go/clear-text-logging] + logger.Info(password) // $ Alert[go/clear-text-logging] + logger.Informational(password) // $ Alert[go/clear-text-logging] + logger.Notice(password) // $ Alert[go/clear-text-logging] + logger.Trace(password) // $ Alert[go/clear-text-logging] + logger.Warn(password) // $ Alert[go/clear-text-logging] + logger.Warning(password) // $ Alert[go/clear-text-logging] + utils.Display(password) // $ Alert[go/clear-text-logging] } type myStruct struct { @@ -196,83 +196,83 @@ func sanitizersTest(ctx *context.Context) { input := ctx.Input output := ctx.Output - untrusted := input.Data()["someKey"] - output.Body([]byte(beego.HTML2str(untrusted.(string)))) - output.Body([]byte(beego.Htmlunquote(untrusted.(string)))) + untrusted := input.Data()["someKey"] // $ Source[go/reflected-xss] + output.Body([]byte(beego.HTML2str(untrusted.(string)))) // $ Alert[go/reflected-xss] + output.Body([]byte(beego.Htmlunquote(untrusted.(string)))) // $ Alert[go/reflected-xss] mapVal, _ := beego.MapGet(untrusted.(map[string][]byte), "somekey") - output.Body(mapVal.([]byte)) - output.Body([]byte(beego.Str2html(untrusted.(string)))) - output.Body([]byte(beego.Substr(untrusted.(string), 1, 2))) + output.Body(mapVal.([]byte)) // $ Alert[go/reflected-xss] + output.Body([]byte(beego.Str2html(untrusted.(string)))) // $ Alert[go/reflected-xss] + output.Body([]byte(beego.Substr(untrusted.(string), 1, 2))) // $ Alert[go/reflected-xss] var s myStruct - beego.ParseForm(ctx.Request.Form, s) - output.Body([]byte(s.field)) + beego.ParseForm(ctx.Request.Form, s) // $ Source[go/reflected-xss] + output.Body([]byte(s.field)) // $ Alert[go/reflected-xss] } // BAD: using user-provided data as paths in file-system operations func fsOpsTest(ctx *context.Context, c *beego.Controller, fs beego.FileSystem) { input := ctx.Input - untrusted := input.Data()["someKey"].(string) - beego.Walk(nil, untrusted, func(path string, info os.FileInfo, err error) error { return nil }) - fs.Open(untrusted) - c.SaveToFile("someReceviedFile", untrusted) + untrusted := input.Data()["someKey"].(string) // $ Source[go/path-injection] + beego.Walk(nil, untrusted, func(path string, info os.FileInfo, err error) error { return nil }) // $ Alert[go/path-injection] + fs.Open(untrusted) // $ Alert[go/path-injection] + c.SaveToFile("someReceviedFile", untrusted) // $ Alert[go/path-injection] } // BAD: echoing untrusted data, using various Controller sources func controllerSourceTest(c *beego.Controller, output *context.BeegoOutput) { - f, fh, _ := c.GetFile("somename") - output.Body([]byte(fh.Filename)) + f, fh, _ := c.GetFile("somename") // $ Source[go/reflected-xss] + output.Body([]byte(fh.Filename)) // $ Alert[go/reflected-xss] content, _ := ioutil.ReadAll(f) - output.Body(content) + output.Body(content) // $ Alert[go/reflected-xss] - files, _ := c.GetFiles("someothername") - output.Body([]byte(files[0].Filename)) + files, _ := c.GetFiles("someothername") // $ Source[go/reflected-xss] + output.Body([]byte(files[0].Filename)) // $ Alert[go/reflected-xss] - s := c.GetString("somekey") - output.Body([]byte(s)) + s := c.GetString("somekey") // $ Source[go/reflected-xss] + output.Body([]byte(s)) // $ Alert[go/reflected-xss] - ss := c.GetStrings("someotherkey") - output.Body([]byte(ss[0])) + ss := c.GetStrings("someotherkey") // $ Source[go/reflected-xss] + output.Body([]byte(ss[0])) // $ Alert[go/reflected-xss] - val := c.Input()["thirdkey"] - output.Body([]byte(val[0])) + val := c.Input()["thirdkey"] // $ Source[go/reflected-xss] + output.Body([]byte(val[0])) // $ Alert[go/reflected-xss] var str myStruct - c.ParseForm(str) - output.Body([]byte(str.field)) + c.ParseForm(str) // $ Source[go/reflected-xss] + output.Body([]byte(str.field)) // $ Alert[go/reflected-xss] } func controllerSinkTest(c *beego.Controller) { - untrusted := c.GetString("somekey") - c.SetData(untrusted) // GOOD: SetData always uses a non-html content-type, so no XSS risk + untrusted := c.GetString("somekey") // $ Source[go/reflected-xss] + c.SetData(untrusted) // GOOD: SetData always uses a non-html content-type, so no XSS risk - c.CustomAbort(500, untrusted) // BAD: CustomAbort doesn't set a content-type, so there is an XSS risk + c.CustomAbort(500, untrusted) // $ Alert[go/reflected-xss] // BAD: CustomAbort doesn't set a content-type, so there is an XSS risk } func redirectTest(c *beego.Controller, ctx *context.Context) { - c.Redirect(c.GetString("somekey"), 304) // BAD: User-controlled redirect - ctx.Redirect(304, c.GetString("somekey")) // BAD: User-controlled redirect + c.Redirect(c.GetString("somekey"), 304) // $ Alert[go/unvalidated-url-redirection] + ctx.Redirect(304, c.GetString("somekey")) // $ Alert[go/unvalidated-url-redirection] } // BAD: echoing untrusted data, using Context source func contextSourceTest(c *context.Context) { - c.Output.Body([]byte(c.GetCookie("somekey"))) + c.Output.Body([]byte(c.GetCookie("somekey"))) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data, using Context sinks func contextSinkTest(c *context.Context) { - c.WriteString(c.GetCookie("somekey")) - c.Abort(500, c.GetCookie("someOtherKey")) + c.WriteString(c.GetCookie("somekey")) // $ Alert[go/reflected-xss] + c.Abort(500, c.GetCookie("someOtherKey")) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data, using context.WriteBody as a propagator func contextWriteBodyTest(c *context.Context) { - context.WriteBody("some/encoding", c.ResponseWriter, []byte(c.GetCookie("someKey"))) + context.WriteBody("some/encoding", c.ResponseWriter, []byte(c.GetCookie("someKey"))) // $ Alert[go/reflected-xss] } // BAD unless otherwise noted: echoing untrusted data, using various utils methods as propagators func testUtilsPropagators(c *beego.Controller) { - files, _ := c.GetFiles("someothername") + files, _ := c.GetFiles("someothername") // $ Source[go/reflected-xss] genericFiles := make([]interface{}, len(files), len(files)) for i := range files { genericFiles[i] = files[i] @@ -280,36 +280,36 @@ func testUtilsPropagators(c *beego.Controller) { untainted := make([]interface{}, 1, 1) - c.CustomAbort(500, utils.GetDisplayString(files[0].Filename)) - c.CustomAbort(500, utils.SliceChunk(genericFiles, 1)[0][0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceDiff(genericFiles, untainted)[0].(*multipart.FileHeader).Filename) + c.CustomAbort(500, utils.GetDisplayString(files[0].Filename)) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceChunk(genericFiles, 1)[0][0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceDiff(genericFiles, untainted)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] // GOOD: the tainted values are subtracted, so taint is not propagated c.CustomAbort(500, utils.SliceDiff(untainted, genericFiles)[0].(*multipart.FileHeader).Filename) c.CustomAbort( 500, utils.SliceFilter( genericFiles, - func([]interface{}) bool { return true })[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceIntersect(genericFiles, untainted)[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceIntersect(untainted, genericFiles)[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceMerge(genericFiles, untainted)[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceMerge(untainted, genericFiles)[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SlicePad(untainted, 10, genericFiles[0])[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SlicePad(genericFiles, 10, untainted[0])[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceRand(genericFiles).(*multipart.FileHeader).Filename) + func([]interface{}) bool { return true })[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceIntersect(genericFiles, untainted)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceIntersect(untainted, genericFiles)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceMerge(genericFiles, untainted)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceMerge(untainted, genericFiles)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SlicePad(untainted, 10, genericFiles[0])[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SlicePad(genericFiles, 10, untainted[0])[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceRand(genericFiles).(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] // Note this is misnamed -- it's a map operation, not a reduce - c.CustomAbort(500, utils.SliceReduce(genericFiles, func(x interface{}) interface{} { return x })[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceShuffle(genericFiles)[0].(*multipart.FileHeader).Filename) - c.CustomAbort(500, utils.SliceUnique(genericFiles)[0].(*multipart.FileHeader).Filename) + c.CustomAbort(500, utils.SliceReduce(genericFiles, func(x interface{}) interface{} { return x })[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceShuffle(genericFiles)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] + c.CustomAbort(500, utils.SliceUnique(genericFiles)[0].(*multipart.FileHeader).Filename) // $ Alert[go/reflected-xss] } // BAD: echoing untrusted data, using BeeMap as an intermediary func testBeeMap(c *beego.Controller) { bMap := utils.NewBeeMap() - untrusted := c.GetString("someKey") + untrusted := c.GetString("someKey") // $ Source[go/reflected-xss] bMap.Set("someKey", untrusted) - c.CustomAbort(500, bMap.Get("someKey").(string)) - c.CustomAbort(500, bMap.Items()["someKey"].(string)) + c.CustomAbort(500, bMap.Get("someKey").(string)) // $ Alert[go/reflected-xss] + c.CustomAbort(500, bMap.Items()["someKey"].(string)) // $ Alert[go/reflected-xss] } // GOOD: using the input URL for a redirect operation @@ -321,25 +321,25 @@ func testSafeRedirects(c *beego.Controller, ctx *context.Context) { // BAD: using RequestBody data as path in a file-system operation func requestBodySourceTest(ctx *context.Context, c *beego.Controller) { var dat map[string]interface{} - json.Unmarshal(ctx.Input.RequestBody, &dat) + json.Unmarshal(ctx.Input.RequestBody, &dat) // $ Source[go/path-injection] untrusted := dat["filepath"].(string) - c.SaveToFile("someReceviedFile", untrusted) + c.SaveToFile("someReceviedFile", untrusted) // $ Alert[go/path-injection] } // BAD: using user-provided data as paths in file-system operations func fsOpsTest2(ctx *context.Context, c *beego.Controller, fs beego.FileSystem) { input := ctx.Input - untrusted := input.Data()["someKey"].(string) + untrusted := input.Data()["someKey"].(string) // $ Source[go/path-injection] beegoOutput := context.BeegoOutput{} - beegoOutput.Download(untrusted, "license.txt") + beegoOutput.Download(untrusted, "license.txt") // $ Alert[go/path-injection] } // BAD: using user-provided data as paths in file-system operations func fsOpsV2Test(ctx *Beegov2Context.Context, c *beegov2.Controller) { input := ctx.Input - untrusted := input.Data()["someKey"].(string) + untrusted := input.Data()["someKey"].(string) // $ Source[go/path-injection] buffer := make([]byte, 10) - _ = c.SaveToFileWithBuffer("filenameExistsInForm", untrusted, buffer) + _ = c.SaveToFileWithBuffer("filenameExistsInForm", untrusted, buffer) // $ Alert[go/path-injection] beegoOutput := Beegov2Context.BeegoOutput{} - beegoOutput.Download(untrusted, "license.txt") + beegoOutput.Download(untrusted, "license.txt") // $ Alert[go/path-injection] } From 1e8f00ef60acb5da05559a821daa710537aff787 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 12 Dec 2025 07:14:34 +0000 Subject: [PATCH 03/11] Fix test for reflected xss sanitizer It used to pass even without ErrorSanitizer because `cookie` is already sanitized. --- .../Security/CWE-079/ReflectedXss.expected | 22 ++++++++++--------- .../Security/CWE-079/reflectedxsstest.go | 8 +++---- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected index 0e1265b5c1e6..b95abaa47c50 100644 --- a/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected +++ b/go/ql/test/query-tests/Security/CWE-079/ReflectedXss.expected @@ -6,8 +6,8 @@ | contenttype.go:79:11:79:14 | data | contenttype.go:73:10:73:28 | call to FormValue | contenttype.go:79:11:79:14 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:73:10:73:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | | | contenttype.go:91:4:91:7 | data | contenttype.go:88:10:88:28 | call to FormValue | contenttype.go:91:4:91:7 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:88:10:88:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | | | contenttype.go:114:50:114:53 | data | contenttype.go:113:10:113:28 | call to FormValue | contenttype.go:114:50:114:53 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:113:10:113:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | | -| reflectedxsstest.go:33:10:33:57 | type conversion | reflectedxsstest.go:31:2:31:44 | ... := ...[0] | reflectedxsstest.go:33:10:33:57 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:31:2:31:44 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | -| reflectedxsstest.go:34:10:34:62 | type conversion | reflectedxsstest.go:31:2:31:44 | ... := ...[1] | reflectedxsstest.go:34:10:34:62 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:31:2:31:44 | ... := ...[1] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | +| reflectedxsstest.go:33:10:33:57 | type conversion | reflectedxsstest.go:30:2:30:44 | ... := ...[0] | reflectedxsstest.go:33:10:33:57 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:30:2:30:44 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | +| reflectedxsstest.go:34:10:34:62 | type conversion | reflectedxsstest.go:30:2:30:44 | ... := ...[1] | reflectedxsstest.go:34:10:34:62 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:30:2:30:44 | ... := ...[1] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | | reflectedxsstest.go:44:10:44:55 | type conversion | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | reflectedxsstest.go:44:10:44:55 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | | reflectedxsstest.go:45:10:45:18 | byteSlice | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | reflectedxsstest.go:45:10:45:18 | byteSlice | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:38:2:38:35 | ... := ...[0] | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | | reflectedxsstest.go:54:11:54:21 | type conversion | reflectedxsstest.go:51:14:51:18 | selection of URL | reflectedxsstest.go:54:11:54:21 | type conversion | Cross-site scripting vulnerability due to $@. | reflectedxsstest.go:51:14:51:18 | selection of URL | user-provided value | reflectedxsstest.go:0:0:0:0 | reflectedxsstest.go | | @@ -30,10 +30,11 @@ edges | contenttype.go:73:10:73:28 | call to FormValue | contenttype.go:79:11:79:14 | data | provenance | Src:MaD:8 | | contenttype.go:88:10:88:28 | call to FormValue | contenttype.go:91:4:91:7 | data | provenance | Src:MaD:8 | | contenttype.go:113:10:113:28 | call to FormValue | contenttype.go:114:50:114:53 | data | provenance | Src:MaD:8 | -| reflectedxsstest.go:31:2:31:44 | ... := ...[0] | reflectedxsstest.go:32:30:32:33 | file | provenance | Src:MaD:7 | -| reflectedxsstest.go:31:2:31:44 | ... := ...[1] | reflectedxsstest.go:34:46:34:60 | selection of Filename | provenance | Src:MaD:7 | -| reflectedxsstest.go:32:2:32:34 | ... := ...[0] | reflectedxsstest.go:33:49:33:55 | content | provenance | | -| reflectedxsstest.go:32:30:32:33 | file | reflectedxsstest.go:32:2:32:34 | ... := ...[0] | provenance | MaD:13 | +| reflectedxsstest.go:30:2:30:44 | ... := ...[0] | reflectedxsstest.go:31:30:31:33 | file | provenance | Src:MaD:7 | +| reflectedxsstest.go:30:2:30:44 | ... := ...[1] | reflectedxsstest.go:34:46:34:60 | selection of Filename | provenance | Src:MaD:7 | +| reflectedxsstest.go:31:2:31:34 | ... := ...[0] | reflectedxsstest.go:32:48:32:54 | content | provenance | | +| reflectedxsstest.go:31:30:31:33 | file | reflectedxsstest.go:31:2:31:34 | ... := ...[0] | provenance | MaD:13 | +| reflectedxsstest.go:32:48:32:54 | content | reflectedxsstest.go:33:49:33:55 | content | provenance | | | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | reflectedxsstest.go:33:17:33:56 | call to Sprintf | provenance | MaD:12 | | reflectedxsstest.go:33:17:33:56 | call to Sprintf | reflectedxsstest.go:33:10:33:57 | type conversion | provenance | | | reflectedxsstest.go:33:49:33:55 | content | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | provenance | | @@ -106,10 +107,11 @@ nodes | contenttype.go:91:4:91:7 | data | semmle.label | data | | contenttype.go:113:10:113:28 | call to FormValue | semmle.label | call to FormValue | | contenttype.go:114:50:114:53 | data | semmle.label | data | -| reflectedxsstest.go:31:2:31:44 | ... := ...[0] | semmle.label | ... := ...[0] | -| reflectedxsstest.go:31:2:31:44 | ... := ...[1] | semmle.label | ... := ...[1] | -| reflectedxsstest.go:32:2:32:34 | ... := ...[0] | semmle.label | ... := ...[0] | -| reflectedxsstest.go:32:30:32:33 | file | semmle.label | file | +| reflectedxsstest.go:30:2:30:44 | ... := ...[0] | semmle.label | ... := ...[0] | +| reflectedxsstest.go:30:2:30:44 | ... := ...[1] | semmle.label | ... := ...[1] | +| reflectedxsstest.go:31:2:31:34 | ... := ...[0] | semmle.label | ... := ...[0] | +| reflectedxsstest.go:31:30:31:33 | file | semmle.label | file | +| reflectedxsstest.go:32:48:32:54 | content | semmle.label | content | | reflectedxsstest.go:33:10:33:57 | type conversion | semmle.label | type conversion | | reflectedxsstest.go:33:17:33:56 | []type{args} [array] | semmle.label | []type{args} [array] | | reflectedxsstest.go:33:17:33:56 | call to Sprintf | semmle.label | call to Sprintf | diff --git a/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go b/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go index b3ddc79535b4..ce708a2c7d78 100644 --- a/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go +++ b/go/ql/test/query-tests/Security/CWE-079/reflectedxsstest.go @@ -25,11 +25,11 @@ func ServeJsonDirect(w http.ResponseWriter, r http.Request) { func ErrTest(w http.ResponseWriter, r http.Request) { cookie, err := r.Cookie("somecookie") - w.Write([]byte(fmt.Sprintf("Cookie result: %v", cookie))) // GOOD: Cookie's value is not user-controlled in reflected xss. - w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless - http.Error(w, fmt.Sprintf("Cookie result: %v", cookie), 500) // Good: only plain text is written. - file, header, err := r.FormFile("someFile") // $ Source[go/reflected-xss] + w.Write([]byte(fmt.Sprintf("Cookie result: %v", cookie))) // GOOD: Cookie's value is not user-controlled in reflected xss. + w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless + file, header, err := r.FormFile("someFile") // $ Source[go/reflected-xss] content, err2 := io.ReadAll(file) + http.Error(w, fmt.Sprintf("File content: %v", content), 500) // Good: only plain text is written. w.Write([]byte(fmt.Sprintf("File content: %v", content))) // $ Alert[go/reflected-xss] // BAD: file content is user-controlled w.Write([]byte(fmt.Sprintf("File name: %v", header.Filename))) // $ Alert[go/reflected-xss] // BAD: file header is user-controlled w.Write([]byte(fmt.Sprintf("FormFile error: %v", err))) // GOOD: FormFile's err return is harmless From 10c5a47662578ab587d33209f82378c251e83bc5 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:36:45 +0000 Subject: [PATCH 04/11] Allow query-specific MaD sanitizers --- .../lib/semmle/go/security/AllocationSizeOverflow.qll | 5 ++++- go/ql/lib/semmle/go/security/CleartextLogging.qll | 2 ++ go/ql/lib/semmle/go/security/CommandInjection.qll | 5 ++++- go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll | 2 ++ go/ql/lib/semmle/go/security/InsecureRandomness.qll | 5 ++++- go/ql/lib/semmle/go/security/LogInjection.qll | 5 ++++- .../semmle/go/security/MissingJwtSignatureCheck.qll | 2 ++ go/ql/lib/semmle/go/security/OpenUrlRedirect.qll | 5 ++++- go/ql/lib/semmle/go/security/ReflectedXss.qll | 5 ++++- go/ql/lib/semmle/go/security/RequestForgery.qll | 5 ++++- go/ql/lib/semmle/go/security/SqlInjection.qll | 4 +++- go/ql/lib/semmle/go/security/StoredCommand.qll | 4 +++- go/ql/lib/semmle/go/security/StoredXss.qll | 5 ++++- go/ql/lib/semmle/go/security/TaintedPath.qll | 5 ++++- .../semmle/go/security/UncontrolledAllocationSize.qll | 5 ++++- go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qll | 5 ++++- go/ql/lib/semmle/go/security/XPathInjection.qll | 5 ++++- go/ql/lib/semmle/go/security/ZipSlip.qll | 5 ++++- .../InconsistentCode/UnhandledCloseWritableHandle.ql | 2 ++ go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql | 2 ++ .../Security/CWE-020/SuspiciousCharacterInRegexp.ql | 2 ++ .../Security/CWE-079/HtmlTemplateEscapingBypassXss.ql | 2 ++ go/ql/src/Security/CWE-209/StackTraceExposure.ql | 2 ++ go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql | 2 ++ go/ql/src/Security/CWE-326/InsufficientKeySize.ql | 3 ++- go/ql/src/Security/CWE-327/InsecureTLS.ql | 4 ++++ go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql | 10 ++++++++-- go/ql/src/Security/CWE-352/ConstantOauth2State.ql | 2 ++ go/ql/src/Security/CWE-601/BadRedirectCheck.ql | 2 ++ go/ql/src/Security/CWE-640/EmailInjection.qll | 2 ++ 30 files changed, 96 insertions(+), 18 deletions(-) diff --git a/go/ql/lib/semmle/go/security/AllocationSizeOverflow.qll b/go/ql/lib/semmle/go/security/AllocationSizeOverflow.qll index 03df08179751..d041f9e0ca01 100644 --- a/go/ql/lib/semmle/go/security/AllocationSizeOverflow.qll +++ b/go/ql/lib/semmle/go/security/AllocationSizeOverflow.qll @@ -45,7 +45,10 @@ module AllocationSizeOverflow { predicate isSink(DataFlow::Node sink) { isSinkWithAllocationSize(sink, _) } - predicate isBarrier(DataFlow::Node nd) { nd instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/allocation-size-overflow") + } predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { additionalStep(pred, succ) diff --git a/go/ql/lib/semmle/go/security/CleartextLogging.qll b/go/ql/lib/semmle/go/security/CleartextLogging.qll index 5254b3e3a298..0e2e555f5d12 100644 --- a/go/ql/lib/semmle/go/security/CleartextLogging.qll +++ b/go/ql/lib/semmle/go/security/CleartextLogging.qll @@ -24,6 +24,8 @@ module CleartextLogging { predicate isBarrier(DataFlow::Node node) { node instanceof Barrier or + barrierNode(node, "go/clear-text-logging") + or exists(DataFlow::CallNode call | node = call.getResult() | call.getTarget() = Builtin::error().getType().getMethod("Error") or diff --git a/go/ql/lib/semmle/go/security/CommandInjection.qll b/go/ql/lib/semmle/go/security/CommandInjection.qll index face45358a91..a12f47c7bf2e 100644 --- a/go/ql/lib/semmle/go/security/CommandInjection.qll +++ b/go/ql/lib/semmle/go/security/CommandInjection.qll @@ -23,7 +23,9 @@ module CommandInjection { exists(Sink s | sink = s | not s.doubleDashIsSanitizing()) } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or barrierNode(node, "go/command-injection") + } predicate observeDiffInformedIncrementalMode() { any() } } @@ -80,6 +82,7 @@ module CommandInjection { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer or + barrierNode(node, "go/command-injection") or node = any(ArgumentArrayWithDoubleDash array).getASanitizedElement() } diff --git a/go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll b/go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll index 8eed50b87913..bb75d28b4267 100644 --- a/go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll +++ b/go/ql/lib/semmle/go/security/CookieWithoutHttpOnly.qll @@ -23,6 +23,8 @@ private module SensitiveCookieNameConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { isSink(sink, _) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/cookie-httponly-not-set") } + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { exists(Http::CookieOptionWrite co | co.getName() = pred and co.getCookieOutput() = succ) } diff --git a/go/ql/lib/semmle/go/security/InsecureRandomness.qll b/go/ql/lib/semmle/go/security/InsecureRandomness.qll index 4dac659eabf9..c8134085d38b 100644 --- a/go/ql/lib/semmle/go/security/InsecureRandomness.qll +++ b/go/ql/lib/semmle/go/security/InsecureRandomness.qll @@ -24,7 +24,10 @@ module InsecureRandomness { predicate isSink(DataFlow::Node sink) { isSinkWithKind(sink, _) } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/insecure-randomness") + } predicate isBarrierOut(DataFlow::Node node) { isSink(node) } diff --git a/go/ql/lib/semmle/go/security/LogInjection.qll b/go/ql/lib/semmle/go/security/LogInjection.qll index 3db7e27c7815..381872173d1d 100644 --- a/go/ql/lib/semmle/go/security/LogInjection.qll +++ b/go/ql/lib/semmle/go/security/LogInjection.qll @@ -20,7 +20,10 @@ module LogInjection { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/log-injection") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll b/go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll index 1f8185d43979..f660ca8aacc9 100644 --- a/go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll +++ b/go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll @@ -20,6 +20,8 @@ module MissingJwtSignatureCheck { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/missing-jwt-signature-check") } + predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { any(AdditionalFlowStep s).step(nodeFrom, nodeTo) } diff --git a/go/ql/lib/semmle/go/security/OpenUrlRedirect.qll b/go/ql/lib/semmle/go/security/OpenUrlRedirect.qll index eb651c3b69fc..80f38b8a5902 100644 --- a/go/ql/lib/semmle/go/security/OpenUrlRedirect.qll +++ b/go/ql/lib/semmle/go/security/OpenUrlRedirect.qll @@ -22,7 +22,10 @@ module OpenUrlRedirect { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Barrier } + predicate isBarrier(DataFlow::Node node) { + node instanceof Barrier or + barrierNode(node, "go/unvalidated-url-redirection") + } predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { // taint steps that do not include flow through fields diff --git a/go/ql/lib/semmle/go/security/ReflectedXss.qll b/go/ql/lib/semmle/go/security/ReflectedXss.qll index 35501269cc1c..57a93dce5d33 100644 --- a/go/ql/lib/semmle/go/security/ReflectedXss.qll +++ b/go/ql/lib/semmle/go/security/ReflectedXss.qll @@ -21,7 +21,10 @@ module ReflectedXss { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/reflected-xss") + } predicate observeDiffInformedIncrementalMode() { any() } diff --git a/go/ql/lib/semmle/go/security/RequestForgery.qll b/go/ql/lib/semmle/go/security/RequestForgery.qll index 03b6f9ac0b00..c2717a51af76 100644 --- a/go/ql/lib/semmle/go/security/RequestForgery.qll +++ b/go/ql/lib/semmle/go/security/RequestForgery.qll @@ -21,7 +21,10 @@ module RequestForgery { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/request-forgery") + } predicate isBarrierOut(DataFlow::Node node) { node instanceof SanitizerEdge } diff --git a/go/ql/lib/semmle/go/security/SqlInjection.qll b/go/ql/lib/semmle/go/security/SqlInjection.qll index 5b7513090258..f031c86fb8e4 100644 --- a/go/ql/lib/semmle/go/security/SqlInjection.qll +++ b/go/ql/lib/semmle/go/security/SqlInjection.qll @@ -22,7 +22,9 @@ module SqlInjection { NoSql::isAdditionalMongoTaintStep(pred, succ) } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or barrierNode(node, "go/sql-injection") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/StoredCommand.qll b/go/ql/lib/semmle/go/security/StoredCommand.qll index 983f739bdab2..34f12abc932b 100644 --- a/go/ql/lib/semmle/go/security/StoredCommand.qll +++ b/go/ql/lib/semmle/go/security/StoredCommand.qll @@ -25,7 +25,9 @@ module StoredCommand { predicate isSink(DataFlow::Node sink) { sink instanceof CommandInjection::Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof CommandInjection::Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof CommandInjection::Sanitizer or barrierNode(node, "go/stored-command") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/StoredXss.qll b/go/ql/lib/semmle/go/security/StoredXss.qll index 3bea8e8c1e00..3206ad9e4cc7 100644 --- a/go/ql/lib/semmle/go/security/StoredXss.qll +++ b/go/ql/lib/semmle/go/security/StoredXss.qll @@ -21,7 +21,10 @@ module StoredXss { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/stored-xss") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/TaintedPath.qll b/go/ql/lib/semmle/go/security/TaintedPath.qll index b814ad5d4ac6..b7e9014343ba 100644 --- a/go/ql/lib/semmle/go/security/TaintedPath.qll +++ b/go/ql/lib/semmle/go/security/TaintedPath.qll @@ -16,7 +16,10 @@ module TaintedPath { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/path-injection") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll b/go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll index 91bbcfaa1edd..8963739b45aa 100644 --- a/go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll +++ b/go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll @@ -18,7 +18,10 @@ module UncontrolledAllocationSize { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/uncontrolled-allocation-size") + } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(Function f, DataFlow::CallNode cn | cn = f.getACall() | diff --git a/go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qll b/go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qll index 0ced26c3eff3..a99ed4ce1e00 100644 --- a/go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qll +++ b/go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qll @@ -43,7 +43,10 @@ module UnsafeUnzipSymlink { predicate isSink(DataFlow::Node sink) { sink instanceof SymlinkSink } - predicate isBarrier(DataFlow::Node node) { node instanceof SymlinkSanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof SymlinkSanitizer or + barrierNode(node, "go/unsafe-unzip-symlink") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/XPathInjection.qll b/go/ql/lib/semmle/go/security/XPathInjection.qll index 900b81053370..93219e4293a4 100644 --- a/go/ql/lib/semmle/go/security/XPathInjection.qll +++ b/go/ql/lib/semmle/go/security/XPathInjection.qll @@ -18,7 +18,10 @@ module XPathInjection { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/xml/xpath-injection") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/lib/semmle/go/security/ZipSlip.qll b/go/ql/lib/semmle/go/security/ZipSlip.qll index 6de2be91509a..597a70749d5c 100644 --- a/go/ql/lib/semmle/go/security/ZipSlip.qll +++ b/go/ql/lib/semmle/go/security/ZipSlip.qll @@ -16,7 +16,10 @@ module ZipSlip { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + barrierNode(node, "go/zipslip") + } predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql b/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql index 25b1c8ae8fc9..04074c13d2aa 100644 --- a/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql +++ b/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql @@ -129,6 +129,8 @@ module UnhandledFileCloseConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { isCloseSink(sink, _) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/unhandled-writable-file-close") } + predicate observeDiffInformedIncrementalMode() { any() } Location getASelectedSourceLocation(DataFlow::Node source) { diff --git a/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql b/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql index f6e3df7d1d91..0aa549b45178 100644 --- a/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql +++ b/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql @@ -100,6 +100,8 @@ module IncompleteHostNameRegexpConfig implements DataFlow::ConfigSig { not regexpGuardsError(sink) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/incomplete-hostname-regexp") } + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { StringOps::Concatenation::taintStep(node1, node2) } diff --git a/go/ql/src/Security/CWE-020/SuspiciousCharacterInRegexp.ql b/go/ql/src/Security/CWE-020/SuspiciousCharacterInRegexp.ql index 96688298ec39..42cd5a9c7b1e 100644 --- a/go/ql/src/Security/CWE-020/SuspiciousCharacterInRegexp.ql +++ b/go/ql/src/Security/CWE-020/SuspiciousCharacterInRegexp.ql @@ -41,6 +41,8 @@ module SuspiciousCharacterInRegexpConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof RegexpPattern } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/suspicious-character-in-regex") } + predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql index 15373ee85edf..5286e105abac 100644 --- a/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql +++ b/go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.ql @@ -83,6 +83,8 @@ module UntrustedToTemplateExecWithConversionConfig implements DataFlow::StateCon predicate isBarrier(DataFlow::Node node) { node instanceof SharedXss::Sanitizer and not node instanceof SharedXss::HtmlTemplateSanitizer or + barrierNode(node, "go/html-template-escaping-bypass-xss") + or node.getType() instanceof NumericType } diff --git a/go/ql/src/Security/CWE-209/StackTraceExposure.ql b/go/ql/src/Security/CWE-209/StackTraceExposure.ql index 408e12b3c15e..696170053def 100644 --- a/go/ql/src/Security/CWE-209/StackTraceExposure.ql +++ b/go/ql/src/Security/CWE-209/StackTraceExposure.ql @@ -61,6 +61,8 @@ module StackTraceExposureConfig implements DataFlow::ConfigSig { | cgn.dominates(node.getBasicBlock()) ) + or + barrierNode(node, "go/stack-trace-exposure") } predicate observeDiffInformedIncrementalMode() { any() } diff --git a/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql b/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql index 87c5a2184b04..99e14636f98c 100644 --- a/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql +++ b/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql @@ -69,6 +69,8 @@ module Config implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { writeIsSink(sink, _) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/insecure-hostkeycallback") } + predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/src/Security/CWE-326/InsufficientKeySize.ql b/go/ql/src/Security/CWE-326/InsufficientKeySize.ql index 6fa421baaeb3..b6350f7b2464 100644 --- a/go/ql/src/Security/CWE-326/InsufficientKeySize.ql +++ b/go/ql/src/Security/CWE-326/InsufficientKeySize.ql @@ -23,7 +23,8 @@ module Config implements DataFlow::ConfigSig { } predicate isBarrier(DataFlow::Node node) { - node = DataFlow::BarrierGuard::getABarrierNode() + node = DataFlow::BarrierGuard::getABarrierNode() or + barrierNode(node, "go/weak-crypto-key") } predicate observeDiffInformedIncrementalMode() { any() } diff --git a/go/ql/src/Security/CWE-327/InsecureTLS.ql b/go/ql/src/Security/CWE-327/InsecureTLS.ql index b5d8a81f3d82..31826d68a409 100644 --- a/go/ql/src/Security/CWE-327/InsecureTLS.ql +++ b/go/ql/src/Security/CWE-327/InsecureTLS.ql @@ -71,6 +71,8 @@ module TlsVersionFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { intIsSource(source, _) } predicate isSink(DataFlow::Node sink) { isSink(sink, _, _, _) } + + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/insecure-tls") } } /** @@ -195,6 +197,8 @@ module TlsInsecureCipherSuitesFlowConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { isSink(sink, _, _, _) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/insecure-tls") } + /** * Declare sinks as out-sanitizers in order to avoid producing superfluous paths where a cipher * is written to CipherSuites, then the list is further extended with either safe or tainted diff --git a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql index 0a38d9729f01..1786b1922938 100644 --- a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql +++ b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql @@ -28,7 +28,10 @@ module NormalHashFunctionFlow { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Barrier } + predicate isBarrier(DataFlow::Node node) { + node instanceof Barrier or + barrierNode(node, "go/weak-sensitive-data-hashing") + } predicate isBarrierIn(DataFlow::Node node) { // make sources barriers so that we only report the closest instance @@ -61,7 +64,10 @@ module ComputationallyExpensiveHashFunctionFlow { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - predicate isBarrier(DataFlow::Node node) { node instanceof Barrier } + predicate isBarrier(DataFlow::Node node) { + node instanceof Barrier or + barrierNode(node, "go/weak-sensitive-data-hashing") + } predicate isBarrierIn(DataFlow::Node node) { // make sources barriers so that we only report the closest instance diff --git a/go/ql/src/Security/CWE-352/ConstantOauth2State.ql b/go/ql/src/Security/CWE-352/ConstantOauth2State.ql index edbb41782b8c..23f23148ed22 100644 --- a/go/ql/src/Security/CWE-352/ConstantOauth2State.ql +++ b/go/ql/src/Security/CWE-352/ConstantOauth2State.ql @@ -41,6 +41,8 @@ module ConstantStateFlowConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { isSinkCall(sink, _) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/constant-oauth2-state") } + predicate observeDiffInformedIncrementalMode() { any() } } diff --git a/go/ql/src/Security/CWE-601/BadRedirectCheck.ql b/go/ql/src/Security/CWE-601/BadRedirectCheck.ql index 7b4cc9f99fcd..7cd0283a9f0d 100644 --- a/go/ql/src/Security/CWE-601/BadRedirectCheck.ql +++ b/go/ql/src/Security/CWE-601/BadRedirectCheck.ql @@ -110,6 +110,8 @@ module Config implements DataFlow::ConfigSig { exists(Write w | w.writesField(node2, _, node1)) } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/bad-redirect-check") } + predicate isBarrierOut(DataFlow::Node node) { // assume this value is safe if something is prepended to it. exists(StringOps::Concatenation conc, int i, int j | i < j | diff --git a/go/ql/src/Security/CWE-640/EmailInjection.qll b/go/ql/src/Security/CWE-640/EmailInjection.qll index e3016b44aa1a..2a3e5f66180c 100644 --- a/go/ql/src/Security/CWE-640/EmailInjection.qll +++ b/go/ql/src/Security/CWE-640/EmailInjection.qll @@ -21,6 +21,8 @@ module EmailInjection { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + predicate isBarrier(DataFlow::Node node) { barrierNode(node, "go/email-injection") } + predicate observeDiffInformedIncrementalMode() { any() } } From 352c07a11776888cfbf6608b24d08760e24ade2b Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:10:08 +0000 Subject: [PATCH 05/11] Allow non-query-specific MaD sanitizers --- .../lib/semmle/go/security/CommandInjectionCustomizations.qll | 4 ++++ go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll | 4 ++++ go/ql/lib/semmle/go/security/Xss.qll | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/go/ql/lib/semmle/go/security/CommandInjectionCustomizations.qll b/go/ql/lib/semmle/go/security/CommandInjectionCustomizations.qll index c2874d7cdac4..bc42a5e59f02 100644 --- a/go/ql/lib/semmle/go/security/CommandInjectionCustomizations.qll +++ b/go/ql/lib/semmle/go/security/CommandInjectionCustomizations.qll @@ -47,6 +47,10 @@ module CommandInjection { override predicate doubleDashIsSanitizing() { exec.doubleDashIsSanitizing() } } + private class ExternalSanitizer extends Sanitizer { + ExternalSanitizer() { barrierNode(this, "command-injection") } + } + /** * A call to a regexp match function, considered as a barrier guard for command injection. */ diff --git a/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll b/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll index 747f2ab0d08d..b46460e1fa27 100644 --- a/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll +++ b/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll @@ -57,6 +57,10 @@ module TaintedPath { PathAsSink() { this = any(FileSystemAccess fsa).getAPathArgument() } } + private class ExternalSanitizer extends Sanitizer { + ExternalSanitizer() { barrierNode(this, "path-injection") } + } + /** * A numeric- or boolean-typed node, considered a sanitizer for path traversal. */ diff --git a/go/ql/lib/semmle/go/security/Xss.qll b/go/ql/lib/semmle/go/security/Xss.qll index f11dc12bf763..0f71ad717ae1 100644 --- a/go/ql/lib/semmle/go/security/Xss.qll +++ b/go/ql/lib/semmle/go/security/Xss.qll @@ -88,6 +88,10 @@ module SharedXss { body.getAContentType().regexpMatch("(?i).*html.*") } + private class ExternalSanitizer extends Sanitizer { + ExternalSanitizer() { barrierNode(this, ["html-injection", "js-injection"]) } + } + /** * A JSON marshaler, acting to sanitize a possible XSS vulnerability because the * marshaled value is very unlikely to be returned as an HTML content-type. From 44c32c6e85040a4e1e870c1c0d98f3c7140e4b68 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:37:26 +0000 Subject: [PATCH 06/11] Convert barrier for cleartext logging to MaD --- go/ql/lib/ext/builtin.model.yml | 5 +++++ go/ql/lib/ext/fmt.model.yml | 5 +++++ go/ql/lib/semmle/go/security/CleartextLogging.qll | 9 +-------- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/go/ql/lib/ext/builtin.model.yml b/go/ql/lib/ext/builtin.model.yml index 816c89008a84..718191b17a10 100644 --- a/go/ql/lib/ext/builtin.model.yml +++ b/go/ql/lib/ext/builtin.model.yml @@ -1,4 +1,9 @@ extensions: + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + - ["", "error", False, "Error", "", "", "ReturnValue", "go/clear-text-logging", "manual"] - addsTo: pack: codeql/go-all extensible: summaryModel diff --git a/go/ql/lib/ext/fmt.model.yml b/go/ql/lib/ext/fmt.model.yml index 2baac68da3a0..7dff5f7ebd66 100644 --- a/go/ql/lib/ext/fmt.model.yml +++ b/go/ql/lib/ext/fmt.model.yml @@ -6,6 +6,11 @@ extensions: - ["fmt", "", False, "Print", "", "", "Argument[0]", "log-injection", "manual"] - ["fmt", "", False, "Printf", "", "", "Argument[0..1]", "log-injection", "manual"] - ["fmt", "", False, "Println", "", "", "Argument[0]", "log-injection", "manual"] + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + - ["fmt", "Stringer", False, "String", "", "", "ReturnValue", "go/clear-text-logging", "manual"] - addsTo: pack: codeql/go-all extensible: summaryModel diff --git a/go/ql/lib/semmle/go/security/CleartextLogging.qll b/go/ql/lib/semmle/go/security/CleartextLogging.qll index 0e2e555f5d12..b7047ffd56a2 100644 --- a/go/ql/lib/semmle/go/security/CleartextLogging.qll +++ b/go/ql/lib/semmle/go/security/CleartextLogging.qll @@ -22,15 +22,8 @@ module CleartextLogging { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } predicate isBarrier(DataFlow::Node node) { - node instanceof Barrier - or + node instanceof Barrier or barrierNode(node, "go/clear-text-logging") - or - exists(DataFlow::CallNode call | node = call.getResult() | - call.getTarget() = Builtin::error().getType().getMethod("Error") - or - call.getTarget().(Method).hasQualifiedName("fmt", "Stringer", "String") - ) } predicate isBarrierIn(DataFlow::Node node) { isSource(node) } From 8aa0fbbb175aa519213dfd3e12ba0c40186f5239 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:15:51 +0000 Subject: [PATCH 07/11] Convert reflected xss sanitizer to MaD --- go/ql/lib/ext/net.http.model.yml | 6 ++++++ .../go/security/ReflectedXssCustomizations.qll | 12 ------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/go/ql/lib/ext/net.http.model.yml b/go/ql/lib/ext/net.http.model.yml index 2bf417cf8f3a..61f9641eb606 100644 --- a/go/ql/lib/ext/net.http.model.yml +++ b/go/ql/lib/ext/net.http.model.yml @@ -7,6 +7,12 @@ extensions: - ["net/http", "", False, "ServeFile", "", "", "Argument[2]", "path-injection", "manual"] # url-redirection - ["net/http", "", False, "Redirect", "", "", "Argument[2]", "url-redirection[0]", "manual"] + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + # Returns the request cookie, which is not user controlled in reflected XSS context. + - ["net/http", "Request", False, "Cookie", "", "", "ReturnValue[0]", "go/reflected-xss", "manual"] - addsTo: pack: codeql/go-all extensible: summaryModel diff --git a/go/ql/lib/semmle/go/security/ReflectedXssCustomizations.qll b/go/ql/lib/semmle/go/security/ReflectedXssCustomizations.qll index 47e823708304..14ccff735146 100644 --- a/go/ql/lib/semmle/go/security/ReflectedXssCustomizations.qll +++ b/go/ql/lib/semmle/go/security/ReflectedXssCustomizations.qll @@ -22,18 +22,6 @@ module ReflectedXss { /** A shared XSS sanitizer as a sanitizer for reflected XSS. */ private class SharedXssSanitizer extends Sanitizer instanceof SharedXss::Sanitizer { } - /** - * A request.Cookie method returns the request cookie, which is not user controlled in reflected XSS context. - */ - class CookieSanitizer extends Sanitizer { - CookieSanitizer() { - exists(Method m, DataFlow::CallNode call | call = m.getACall() | - m.hasQualifiedName("net/http", "Request", "Cookie") and - this = call.getResult(0) - ) - } - } - /** * DEPRECATED: Use `ActiveThreatModelSource` or `Source` instead. */ From c94847fc064883505074f9a9947c5ecfdeb3a73f Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:16:52 +0000 Subject: [PATCH 08/11] Convert xss sanitizer to MaD --- go/ql/lib/ext/github.com.beego.beego.server.web.model.yml | 5 +++++ go/ql/lib/semmle/go/frameworks/Beego.qll | 8 -------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/go/ql/lib/ext/github.com.beego.beego.server.web.model.yml b/go/ql/lib/ext/github.com.beego.beego.server.web.model.yml index 0c539522c5a7..8588af2652c9 100644 --- a/go/ql/lib/ext/github.com.beego.beego.server.web.model.yml +++ b/go/ql/lib/ext/github.com.beego.beego.server.web.model.yml @@ -50,3 +50,8 @@ extensions: - ["group:beego", "Controller", True, "GetString", "", "", "ReturnValue[0]", "remote", "manual"] - ["group:beego", "Controller", True, "GetStrings", "", "", "ReturnValue[0]", "remote", "manual"] - ["group:beego", "Controller", True, "Input", "", "", "ReturnValue[0]", "remote", "manual"] + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + - ["group:beego", "", True, "Htmlquote", "", "", "ReturnValue", "html-injection", "manual"] diff --git a/go/ql/lib/semmle/go/frameworks/Beego.qll b/go/ql/lib/semmle/go/frameworks/Beego.qll index 952958cebf0e..383be8ec42ab 100644 --- a/go/ql/lib/semmle/go/frameworks/Beego.qll +++ b/go/ql/lib/semmle/go/frameworks/Beego.qll @@ -165,14 +165,6 @@ module Beego { override string getAContentType() { none() } } - private class HtmlQuoteSanitizer extends SharedXss::Sanitizer { - HtmlQuoteSanitizer() { - exists(DataFlow::CallNode c | c.getTarget().hasQualifiedName(packagePath(), "Htmlquote") | - this = c.getArgument(0) - ) - } - } - private class UtilsTaintPropagators extends TaintTracking::FunctionModel { UtilsTaintPropagators() { this.hasQualifiedName(utilsPackagePath(), "GetDisplayString") } From b575195467ca122e87741dde3192d9ca49f6246b Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:19:56 +0000 Subject: [PATCH 09/11] Convert 3 barriers for path injection to MaD --- go/ql/lib/ext/mime.multipart.model.yml | 17 +++++++ go/ql/lib/ext/path.filepath.model.yml | 5 ++ .../go/security/TaintedPathCustomizations.qll | 51 ------------------- 3 files changed, 22 insertions(+), 51 deletions(-) diff --git a/go/ql/lib/ext/mime.multipart.model.yml b/go/ql/lib/ext/mime.multipart.model.yml index 410eac26af69..134481dfce33 100644 --- a/go/ql/lib/ext/mime.multipart.model.yml +++ b/go/ql/lib/ext/mime.multipart.model.yml @@ -1,4 +1,21 @@ extensions: + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + # The only way to create a `mime/multipart.FileHeader` is to create a + # `mime/multipart.Form`, which creates the `Filename` field of each + # `mime/multipart.FileHeader` by calling `Part.FileName`, which calls + # `path/filepath.Base` on its return value. In general `path/filepath.Base` + # is not a sanitizer for path traversal, but in this specific case where the + # output is going to be used as a filename rather than a directory name, it + # is adequate. + - ["mime/multipart", "FileHeader", False, "Filename", "", "", "", "path-injection", "manual"] + # `Part.FileName` calls `path/filepath.Base` on its return value. In + # general `path/filepath.Base` is not a sanitizer for path traversal, but in + # this specific case where the output is going to be used as a filename + # rather than a directory name, it is adequate. + - ["mime/multipart", "Part", False, "FileName", "", "", "ReturnValue", "path-injection", "manual"] - addsTo: pack: codeql/go-all extensible: summaryModel diff --git a/go/ql/lib/ext/path.filepath.model.yml b/go/ql/lib/ext/path.filepath.model.yml index 15bcb7d386d8..d450e2bbc568 100644 --- a/go/ql/lib/ext/path.filepath.model.yml +++ b/go/ql/lib/ext/path.filepath.model.yml @@ -1,4 +1,9 @@ extensions: + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + - ["path/filepath", "", False, "Rel", "", "", "ReturnValue", "path-injection", "manual"] - addsTo: pack: codeql/go-all extensible: summaryModel diff --git a/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll b/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll index b46460e1fa27..20341159c64c 100644 --- a/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll +++ b/go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll @@ -70,19 +70,6 @@ module TaintedPath { } } - /** - * A call to `filepath.Rel`, considered as a sanitizer for path traversal. - */ - class FilepathRelSanitizer extends Sanitizer { - FilepathRelSanitizer() { - exists(Function f, FunctionOutput outp | - f.hasQualifiedName("path/filepath", "Rel") and - outp.isResult(0) and - this = outp.getNode(f.getACall()) - ) - } - } - /** * A call to `filepath.Clean("/" + e)`, considered to sanitize `e` against path traversal. */ @@ -116,44 +103,6 @@ module TaintedPath { } } - /** - * A read from the field `Filename` of the type `mime/multipart.FileHeader`, - * considered as a sanitizer for path traversal. - * - * The only way to create a `mime/multipart.FileHeader` is to create a - * `mime/multipart.Form`, which creates the `Filename` field of each - * `mime/multipart.FileHeader` by calling `Part.FileName`, which calls - * `path/filepath.Base` on its return value. In general `path/filepath.Base` - * is not a sanitizer for path traversal, but in this specific case where the - * output is going to be used as a filename rather than a directory name, it - * is adequate. - */ - class MimeMultipartFileHeaderFilenameSanitizer extends Sanitizer { - MimeMultipartFileHeaderFilenameSanitizer() { - this.(DataFlow::FieldReadNode) - .getField() - .hasQualifiedName("mime/multipart", "FileHeader", "Filename") - } - } - - /** - * A call to `mime/multipart.Part.FileName`, considered as a sanitizer - * against path traversal. - * - * `Part.FileName` calls `path/filepath.Base` on its return value. In - * general `path/filepath.Base` is not a sanitizer for path traversal, but in - * this specific case where the output is going to be used as a filename - * rather than a directory name, it is adequate. - */ - class MimeMultipartPartFileNameSanitizer extends Sanitizer { - MimeMultipartPartFileNameSanitizer() { - this = - any(Method m | m.hasQualifiedName("mime/multipart", "Part", "FileName")) - .getACall() - .getResult() - } - } - /** * A check of the form `!strings.Contains(nd, "..")`, considered as a sanitizer guard for * path traversal. From 9d00fd73e3a8c15743603433a13c2babfd468445 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:22:02 +0000 Subject: [PATCH 10/11] Convert one zip slip barrier to MaD --- go/ql/lib/ext/archive.zip.model.yml | 5 +++++ .../lib/semmle/go/security/ZipSlipCustomizations.qll | 12 ------------ 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/go/ql/lib/ext/archive.zip.model.yml b/go/ql/lib/ext/archive.zip.model.yml index 17e2bb0bd789..5fca52b3eff6 100644 --- a/go/ql/lib/ext/archive.zip.model.yml +++ b/go/ql/lib/ext/archive.zip.model.yml @@ -1,4 +1,9 @@ extensions: + - addsTo: + pack: codeql/go-all + extensible: barrierModel + data: + - ["archive/zip", "File", True, "Open", "", "", "ReturnValue[0]", "go/zipslip", "manual"] - addsTo: pack: codeql/go-all extensible: summaryModel diff --git a/go/ql/lib/semmle/go/security/ZipSlipCustomizations.qll b/go/ql/lib/semmle/go/security/ZipSlipCustomizations.qll index 980c601582e8..19de9e0bcf94 100644 --- a/go/ql/lib/semmle/go/security/ZipSlipCustomizations.qll +++ b/go/ql/lib/semmle/go/security/ZipSlipCustomizations.qll @@ -53,18 +53,6 @@ module ZipSlip { } } - /** - * A zipped file, excluded from for zip slip. - */ - class ZipFileOpen extends Sanitizer { - ZipFileOpen() { - this = - any(DataFlow::MethodCallNode mcn | - mcn.getTarget().hasQualifiedName("archive/zip", "File", "Open") - ).getResult(0) - } - } - /** A path-traversal sink, considered as a taint sink for zip slip. */ class TaintedPathSinkAsSink extends Sink instanceof TaintedPath::Sink { TaintedPathSinkAsSink() { From d9d12cfb045cb92935982defcc9b997f0a85301c Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 16 Dec 2025 17:10:23 +0000 Subject: [PATCH 11/11] (Misc) typo in comment --- go/ql/src/Security/CWE-209/StackTraceExposure.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/ql/src/Security/CWE-209/StackTraceExposure.ql b/go/ql/src/Security/CWE-209/StackTraceExposure.ql index 696170053def..b342a2a8bae2 100644 --- a/go/ql/src/Security/CWE-209/StackTraceExposure.ql +++ b/go/ql/src/Security/CWE-209/StackTraceExposure.ql @@ -53,7 +53,7 @@ module StackTraceExposureConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof Http::ResponseBody } predicate isBarrier(DataFlow::Node node) { - // Sanitise everything controlled by an is-debug-mode check. + // Sanitize everything controlled by an is-debug-mode check. // Imprecision: I don't try to guess which arm of a branch is intended // to mean debug mode, and which is production mode. exists(ControlFlow::ConditionGuardNode cgn |