From dc6a8917a47cf06c69b87ddb11d98c8b774eb8f3 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Thu, 20 Feb 2020 13:19:42 +0000 Subject: [PATCH] Add missing QLDoc for public elements. --- ql/src/RedundantCode/Clones.qll | 21 +++++++-- ql/src/semmle/go/Concepts.qll | 12 ++---- ql/src/semmle/go/Expr.qll | 9 ++-- ql/src/semmle/go/Scopes.qll | 7 ++- ql/src/semmle/go/Stmt.qll | 12 +++--- ql/src/semmle/go/StringOps.qll | 7 ++- .../go/controlflow/ControlFlowGraph.qll | 5 ++- .../go/controlflow/ControlFlowGraphImpl.qll | 16 +++++++ ql/src/semmle/go/controlflow/IR.qll | 9 ++-- ql/src/semmle/go/dataflow/SSA.qll | 3 ++ ql/src/semmle/go/dataflow/SsaImpl.qll | 16 +++++++ .../go/dataflow/internal/DataFlowUtil.qll | 43 +++++++++++++------ .../dataflow/internal/TaintTrackingUtil.qll | 5 ++- .../security/ReflectedXssCustomizations.qll | 7 ++- .../go/security/StringBreakCustomizations.qll | 5 +++ 15 files changed, 129 insertions(+), 48 deletions(-) diff --git a/ql/src/RedundantCode/Clones.qll b/ql/src/RedundantCode/Clones.qll index beccbb9d1ec..a5bcea5a3a1 100644 --- a/ql/src/RedundantCode/Clones.qll +++ b/ql/src/RedundantCode/Clones.qll @@ -74,6 +74,7 @@ class HashableNode extends AstNode { class HashableNullaryNode extends HashableNode { HashableNullaryNode() { not exists(getAChild()) } + /** Holds if this node has the given `kind` and `value`. */ predicate unpack(int kind, string value) { kind = getKind() and value = getValue() } override HashedNode hash() { @@ -87,6 +88,7 @@ class HashableNullaryNode extends HashableNode { class HashableUnaryNode extends HashableNode { HashableUnaryNode() { getNumChild() = 1 and exists(getChild(0)) } + /** Holds if this node has the given `kind` and `value`, and `child` is its only child. */ predicate unpack(int kind, string value, HashedNode child) { kind = getKind() and value = getValue() and child = getChild(0).(HashableNode).hash() } @@ -104,6 +106,7 @@ class HashableUnaryNode extends HashableNode { class HashableBinaryNode extends HashableNode { HashableBinaryNode() { getNumChild() = 2 and exists(getChild(0)) and exists(getChild(1)) } + /** Holds if this node has the given `kind` and `value`, and `left` and `right` are its children. */ predicate unpack(int kind, string value, HashedNode left, HashedNode right) { kind = getKind() and value = getValue() and @@ -128,10 +131,12 @@ class HashableNAryNode extends HashableNode { exists(int n | n = strictcount(getAChild()) | n > 2 or not exists(getChild([0 .. n - 1]))) } + /** Holds if this node has the given `kind`, `value`, and `children`. */ predicate unpack(int kind, string value, HashedChildren children) { kind = getKind() and value = getValue() and children = hashChildren() } + /** Holds if `child` is the `i`th child of this node, and `rest` are its subsequent children. */ predicate childAt(int i, HashedNode child, HashedChildren rest) { child = getChild(i).(HashableNode).hash() and rest = hashChildren(i + 1) } @@ -142,9 +147,11 @@ class HashableNAryNode extends HashableNode { ) } - HashedChildren hashChildren() { result = hashChildren(0) } + /** Gets the hash of this node's children. */ + private HashedChildren hashChildren() { result = hashChildren(0) } - HashedChildren hashChildren(int i) { + /** Gets the hash of this node's children, starting with the `i`th child. */ + private HashedChildren hashChildren(int i) { i = max(int n | exists(getChild(n))) + 1 and result = Nil() or exists(HashedNode child, HashedChildren rest | childAt(i, child, rest) | @@ -153,6 +160,13 @@ class HashableNAryNode extends HashableNode { } } +/** + * A normalized ("hashed") representation of an AST node. + * + * The normalized representation only captures the tree structure of the AST, discarding + * any information about source location or node identity. Thus, if two parts of the AST + * have identical hashes they are structurally identical. + */ newtype HashedNode = MkHashedNullaryNode(int kind, string value) { any(HashableNullaryNode nd).unpack(kind, value) } or MkHashedUnaryNode(int kind, string value, HashedNode child) { @@ -165,7 +179,8 @@ newtype HashedNode = any(HashableNAryNode nd).unpack(kind, value, children) } -newtype HashedChildren = +/** A normalized ("hashed") representation of some or all of the children of a node. */ +private newtype HashedChildren = Nil() or AHashedChild(int i, HashedNode child, HashedChildren rest) { exists(HashableNAryNode nd | nd.childAt(i, child, rest)) diff --git a/ql/src/semmle/go/Concepts.qll b/ql/src/semmle/go/Concepts.qll index ff004393747..03950b66654 100644 --- a/ql/src/semmle/go/Concepts.qll +++ b/ql/src/semmle/go/Concepts.qll @@ -260,13 +260,12 @@ class RegexpMatchFunction extends Function { } module RegexpMatchFunction { - /* + /** * A function that matches a regexp with a string or byte slice. * * Extend this class to model new APIs. If you want to refine existing API models, * extend `RegexpPattern' instead. */ - abstract class Range extends Function { /** * Gets the function input that is the regexp being matched. @@ -415,13 +414,12 @@ module HTTP { } } - /* + /** * A data-flow node that represents a write to an HTTP header. * * Extend this class to refine existing API models. If you want to model new APIs, * extend `HTTP::HeaderWrite::Range` instead. */ - class HeaderWrite extends DataFlow::ExprNode { HeaderWrite::Range self; @@ -490,26 +488,24 @@ module HTTP { } module ResponseBody { - /* + /** * An expression which is written to an HTTP response body. * * Extend this class to model new APIs. If you want to refine existing API models, * extend `HTTP::ResponseBody` instead. */ - abstract class Range extends DataFlow::Node { /** Gets the response writer associated with this header write, if any. */ abstract ResponseWriter getResponseWriter(); } } - /* + /** * An expression which is written to an HTTP response body. * * Extend this class to refine existing API models. If you want to model new APIs, * extend `HTTP::ResponseBody::Range` instead. */ - class ResponseBody extends DataFlow::Node { ResponseBody::Range self; diff --git a/ql/src/semmle/go/Expr.qll b/ql/src/semmle/go/Expr.qll index bb2e16eb9e1..8329515ad77 100644 --- a/ql/src/semmle/go/Expr.qll +++ b/ql/src/semmle/go/Expr.qll @@ -312,6 +312,7 @@ class StructLit extends CompositeLit { StructLit() { st = getType().getUnderlyingType() } + /** Gets the type of this literal. */ StructType getStructType() { result = st } } @@ -324,9 +325,7 @@ class ParenExpr extends @parenexpr, Expr { override Expr stripParens() { result = getExpr().stripParens() } - override predicate isPlatformIndependentConstant() { - getExpr().isPlatformIndependentConstant() - } + override predicate isPlatformIndependentConstant() { getExpr().isPlatformIndependentConstant() } override string toString() { result = "(...)" } } @@ -401,9 +400,7 @@ class TypeAssertExpr extends @typeassertexpr, Expr { override predicate mayHaveOwnSideEffects() { any() } - override predicate isPlatformIndependentConstant() { - getExpr().isPlatformIndependentConstant() - } + override predicate isPlatformIndependentConstant() { getExpr().isPlatformIndependentConstant() } override string toString() { result = "type assertion" } } diff --git a/ql/src/semmle/go/Scopes.qll b/ql/src/semmle/go/Scopes.qll index 9e64ec2bea4..59a77a0fd4b 100644 --- a/ql/src/semmle/go/Scopes.qll +++ b/ql/src/semmle/go/Scopes.qll @@ -140,7 +140,11 @@ class Entity extends @object { or // otherwise fall back on dummy location not exists(getDeclaration()) and - filepath = "" and startline = 0 and startcolumn = 0 and endline = 0 and endcolumn = 0 + filepath = "" and + startline = 0 and + startcolumn = 0 and + endline = 0 and + endcolumn = 0 } } @@ -259,6 +263,7 @@ class Field extends Variable { Field() { fieldstructs(this, declaringType) } + /** Gets the struct type declaring this field. */ StructType getDeclaringType() { result = declaringType } /** diff --git a/ql/src/semmle/go/Stmt.qll b/ql/src/semmle/go/Stmt.qll index a57373229c9..8a99c39f8ce 100644 --- a/ql/src/semmle/go/Stmt.qll +++ b/ql/src/semmle/go/Stmt.qll @@ -449,11 +449,8 @@ class SwitchStmt extends @switchstmt, Stmt, ScopeNode { /** Gets the `i`th non-default case clause of this `switch` statement (0-based). */ CaseClause getNonDefaultCase(int i) { - result = rank[i + 1](CaseClause cc, int j | - cc = getCase(j) and exists(cc.getExpr(_)) - | - cc order by j - ) + result = + rank[i + 1](CaseClause cc, int j | cc = getCase(j) and exists(cc.getExpr(_)) | cc order by j) } /** Gets a non-default case clause of this `switch` statement. */ @@ -555,15 +552,18 @@ class SelectStmt extends @selectstmt, Stmt { /** Gets the `i`th `case` clause in this `select` statement. */ CommClause getNonDefaultCommClause(int i) { - result = rank[i + 1](CommClause cc, int j | + result = + rank[i + 1](CommClause cc, int j | cc = getCommClause(j) and exists(cc.getComm()) | cc order by j ) } + /** Gets the number of `case` clauses in this `select` statement. */ int getNumNonDefaultCommClause() { result = count(getNonDefaultCommClause(_)) } + /** Gets the `default` clause in this `select` statement, if any. */ CommClause getDefaultCommClause() { result = getCommClause(_) and not exists(result.getComm()) diff --git a/ql/src/semmle/go/StringOps.qll b/ql/src/semmle/go/StringOps.qll index 58b7d6d982b..da578fe615b 100644 --- a/ql/src/semmle/go/StringOps.qll +++ b/ql/src/semmle/go/StringOps.qll @@ -216,7 +216,10 @@ module StringOps { */ pragma[noinline] private string getFormatComponentRegex() { - exists(string literal, string opt_flag, string width, string prec, string opt_width_and_prec, string operator, string verb | + exists( + string literal, string opt_flag, string width, string prec, string opt_width_and_prec, + string operator, string verb + | literal = "([^%]|%%)+" and opt_flag = "[-+ #0]?" and width = "\\d+|\\*" and @@ -297,7 +300,7 @@ module StringOps { predicate taintStep(DataFlow::Node src, DataFlow::Node dst) { taintStep(src, dst, _, _) } } - newtype TConcatenationElement = + private newtype TConcatenationElement = /** A root concatenation element that is not itself an operand of a string concatenation. */ MkConcatenationRoot(Concatenation cat) { not cat = any(Concatenation parent).getOperand(_) } or /** A concatenation element that is an operand of a string concatenation. */ diff --git a/ql/src/semmle/go/controlflow/ControlFlowGraph.qll b/ql/src/semmle/go/controlflow/ControlFlowGraph.qll index fd965d35151..e56ca535e5f 100644 --- a/ql/src/semmle/go/controlflow/ControlFlowGraph.qll +++ b/ql/src/semmle/go/controlflow/ControlFlowGraph.qll @@ -7,9 +7,11 @@ private import ControlFlowGraphImpl /** Provides helper predicates for mapping btween CFG nodes and the AST. */ module ControlFlow { + /** A file or function with which a CFG is associated. */ class Root extends AstNode { Root() { exists(this.(File).getADecl()) or exists(this.(FuncDef).getBody()) } + /** Holds if `nd` belongs to this file or function. */ predicate isRootOf(AstNode nd) { this = nd.getEnclosingFunction() or @@ -17,8 +19,10 @@ module ControlFlow { this = nd.getFile() } + /** Gets the synthetic entry node of the CFG for this file or function. */ EntryNode getEntryNode() { result = ControlFlow::entryNode(this) } + /** Gets the synthetic exit node of the CFG for this file or function. */ ExitNode getExitNode() { result = ControlFlow::exitNode(this) } } @@ -147,7 +151,6 @@ module ControlFlow { */ class ConditionGuardNode extends IR::Instruction, MkConditionGuardNode { Expr cond; - boolean outcome; ConditionGuardNode() { this = MkConditionGuardNode(cond, outcome) } diff --git a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll index d86f717dbc9..37216e53532 100644 --- a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -6,6 +6,7 @@ import go +/** A block statement that is not the body of a `switch` or `select` statement. */ class PlainBlock extends BlockStmt { PlainBlock() { not this = any(SwitchStmt sw).getBody() and not this = any(SelectStmt sel).getBody() @@ -320,6 +321,13 @@ newtype TWriteTarget = /** A write target for a returned expression, viewed as a write to the corresponding result variable. */ MkResultWriteTarget(MkResultWriteNode w) +/** + * A control-flow node that represents a no-op. + * + * These control-flow nodes correspond to Go statements that have no runtime semantics other than + * potentially influencing control flow: the branching statements `continue`, `break`, + * `fallthrough` and `goto`; empty blocks; empty statements; and import and type declarations. + */ class SkipNode extends ControlFlow::Node, MkSkipNode { AstNode skip; @@ -336,6 +344,9 @@ class SkipNode extends ControlFlow::Node, MkSkipNode { } } +/** + * A control-flow node that represents the start of the execution of a function or file. + */ class EntryNode extends ControlFlow::Node, MkEntryNode { ControlFlow::Root root; @@ -354,6 +365,9 @@ class EntryNode extends ControlFlow::Node, MkEntryNode { } } +/** + * A control-flow node that represents the end of the execution of a function or file. + */ class ExitNode extends ControlFlow::Node, MkExitNode { ControlFlow::Root root; @@ -1880,9 +1894,11 @@ module CFG { result = MkSkipNode(e) } + /** Holds if evaluation of `root` may start at `first`. */ cached predicate firstNode(ControlFlowTree root, ControlFlow::Node first) { root.firstNode(first) } + /** Holds if evaluation of `root` may complete normally after `last`. */ cached predicate lastNode(ControlFlowTree root, ControlFlow::Node last) { lastNode(root, last, normalCompletion()) diff --git a/ql/src/semmle/go/controlflow/IR.qll b/ql/src/semmle/go/controlflow/IR.qll index b3c655b12b7..9787aff4591 100644 --- a/ql/src/semmle/go/controlflow/IR.qll +++ b/ql/src/semmle/go/controlflow/IR.qll @@ -629,6 +629,7 @@ module IR { ExtractTupleElementInstruction() { this = MkExtractNode(s, i) } + /** Gets the instruction computing the tuple value from which one value is extracted. */ Instruction getBase() { exists(Expr baseExpr | baseExpr = s.(Assignment).getRhs() or @@ -655,12 +656,8 @@ module IR { exists(Type rangeType | rangeType = s.(RangeStmt).getDomain().getType().getUnderlyingType() | exists(Type baseType | baseType = rangeType.(ArrayType).getElementType() or - baseType = rangeType - .(PointerType) - .getBaseType() - .getUnderlyingType() - .(ArrayType) - .getElementType() or + baseType = + rangeType.(PointerType).getBaseType().getUnderlyingType().(ArrayType).getElementType() or baseType = rangeType.(SliceType).getElementType() | i = 0 and diff --git a/ql/src/semmle/go/dataflow/SSA.qll b/ql/src/semmle/go/dataflow/SSA.qll index 5801dd779f0..9accc969346 100644 --- a/ql/src/semmle/go/dataflow/SSA.qll +++ b/ql/src/semmle/go/dataflow/SSA.qll @@ -152,10 +152,12 @@ class SsaDefinition extends TSsaDefinition { * An SSA definition that corresponds to an explicit assignment or other variable definition. */ class SsaExplicitDefinition extends SsaDefinition, TExplicitDef { + /** Gets the instruction where the definition happens. */ IR::Instruction getInstruction() { exists(BasicBlock bb, int i | this = TExplicitDef(bb, i, _) | result = bb.getNode(i)) } + /** Gets the right-hand side of the definition. */ IR::Instruction getRhs() { getInstruction().writes(_, result) } override predicate definesAt(ReachableBasicBlock bb, int i, SsaSourceVariable v) { @@ -314,6 +316,7 @@ private IR::Instruction accessPathAux(TSsaWithFields base, Field f) { ) } +/** An SSA variable with zero or more fields read from it. */ class SsaWithFields extends TSsaWithFields { /** * Gets the SSA variable corresponding to the base of this SSA variable with fields. diff --git a/ql/src/semmle/go/dataflow/SsaImpl.qll b/ql/src/semmle/go/dataflow/SsaImpl.qll index b00dd4bc55e..a15e8595b99 100644 --- a/ql/src/semmle/go/dataflow/SsaImpl.qll +++ b/ql/src/semmle/go/dataflow/SsaImpl.qll @@ -36,14 +36,29 @@ private module Internal { */ cached newtype TSsaDefinition = + /** + * An SSA definition that corresponds to an explicit assignment or other variable definition. + */ TExplicitDef(ReachableBasicBlock bb, int i, SsaSourceVariable v) { defAt(bb, i, v) and (liveAfterDef(bb, i, v) or v.isCaptured()) } or + /** + * An SSA definition representing the capturing of an SSA-convertible variable + * in the closure of a nested function. + * + * Capturing definitions appear at the beginning of such functions, as well as + * at any function call that may affect the value of the variable. + */ TCapture(ReachableBasicBlock bb, int i, SsaSourceVariable v) { mayCapture(bb, i, v) and liveAfterDef(bb, i, v) } or + /** + * An SSA phi node, that is, a pseudo-definition for a variable at a point + * in the flow graph where otherwise two or more definitions for the variable + * would be visible. + */ TPhi(ReachableJoinBlock bb, SsaSourceVariable v) { liveAtEntry(bb, v) and inDefDominanceFrontier(bb, v) @@ -276,4 +291,5 @@ private module Internal { rewindReads(bb, i, v) = 1 and result = getDefReachingEndOf(bb.getImmediateDominator(), v) } } + import Internal diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index 94fc150caef..eebdf7a9559 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -150,6 +150,9 @@ class InstructionNode extends Node, MkInstructionNode { } } +/** + * An expression, viewed as a node in a data flow graph. + */ class ExprNode extends InstructionNode { override IR::EvalInstruction insn; Expr expr; @@ -158,6 +161,7 @@ class ExprNode extends InstructionNode { override Expr asExpr() { result = expr } + /** Gets the underlying expression this node corresponds to. */ Expr getExpr() { result = expr } } @@ -222,9 +226,7 @@ class GlobalFunctionNode extends FunctionNode, MkGlobalFunctionNode { GlobalFunctionNode() { this = MkGlobalFunctionNode(func) } - override ParameterNode getParameter(int i) { - result = parameterNode(func.getParameter(i)) - } + override ParameterNode getParameter(int i) { result = parameterNode(func.getParameter(i)) } override string getName() { result = func.getName() } @@ -264,9 +266,7 @@ class CallNode extends ExprNode { /** Gets the declared target of this call */ Function getTarget() { result = expr.getTarget() } - private DataFlow::Node getACalleeSource() { - result.getASuccessor*() = getCalleeNode() - } + private DataFlow::Node getACalleeSource() { result.getASuccessor*() = getCalleeNode() } /** * Gets the definition of a possible target of this call. @@ -339,9 +339,7 @@ class CallNode extends ExprNode { Node getResult() { not getType() instanceof TupleType and result = this } /** Gets the data flow node corresponding to the receiver of this call, if any. */ - Node getReceiver() { - result = getACalleeSource().(MethodReadNode).getReceiver() - } + Node getReceiver() { result = getACalleeSource().(MethodReadNode).getReceiver() } } /** A data flow node that represents a call to a method. */ @@ -360,8 +358,10 @@ class ReceiverNode extends SsaNode { ReceiverNode() { ssa.getInstruction() = IR::initRecvInstruction(recv) } + /** Gets the receiver variable this node initializes. */ ReceiverVariable asReceiverVariable() { result = recv } + /** Holds if this node initializes the receiver of `fd`. */ predicate isReceiverOf(FuncDef fd) { recv = fd.(MethodDecl).getReceiver() } } @@ -372,8 +372,10 @@ class ParameterNode extends SsaNode { ParameterNode() { ssa.getInstruction() = IR::initParamInstruction(parm) } + /** Gets the parameter this node initializes. */ override Parameter asParameter() { result = parm } + /** Holds if this node initializes the `i`th parameter of `fd`. */ predicate isParameterOf(FuncDef fd, int i) { parm = fd.getParameter(i) } } @@ -464,6 +466,10 @@ class ResultNode extends InstructionNode { } } +/** + * A data-flow node that reads the value of a variable, constant, field or array element, + * or refers to a function. + */ class ReadNode extends InstructionNode { override IR::ReadInstruction insn; @@ -581,7 +587,7 @@ class BinaryOperationNode extends Node { } /** - * An IR instruction corresponding to an expression with a unary operator. + * A data-flow node corresponding to an expression with a unary operator. */ class UnaryOperationNode extends InstructionNode { UnaryOperationNode() { @@ -621,7 +627,7 @@ class UnaryOperationNode extends InstructionNode { } /** - * A pointer-dereference instruction. + * A data-flow node that dereferences a pointer. */ class PointerDereferenceNode extends UnaryOperationNode { PointerDereferenceNode() { @@ -634,29 +640,41 @@ class PointerDereferenceNode extends UnaryOperationNode { } /** - * An address-of instruction. + * A data-flow node that takes the address of a memory location. */ class AddressOperationNode extends UnaryOperationNode, ExprNode { override AddressExpr expr; } +/** + * A data-flow node that reads the value of a field. + */ class FieldReadNode extends ReadNode { override IR::FieldReadInstruction insn; + /** Gets the base node from which the field is read. */ Node getBase() { result = instructionNode(insn.getBase()) } + /** Gets the field this node reads. */ Field getField() { result = insn.getField() } + /** Gets the name of the field this node reads. */ string getFieldName() { result = this.getField().getName() } } +/** + * A data-flow node that refers to a method. + */ class MethodReadNode extends ReadNode { override IR::MethodReadInstruction insn; + /** Gets the receiver node on which the method is referenced. */ Node getReceiver() { result = instructionNode(insn.getReceiver()) } + /** Gets the method this node refers to. */ Method getMethod() { result = insn.getMethod() } + /** Gets the name of the method this node refers to. */ string getMethodName() { result = this.getMethod().getName() } } @@ -701,6 +719,7 @@ class EqualityTestNode extends BinaryOperationNode, ExprNode { * values should be modeled by `TaintTracking::FunctionModel` instead. */ abstract class FunctionModel extends Function { + /** Holds if data flows through this function from `input` to `output`. */ abstract predicate hasDataFlow(FunctionInput input, FunctionOutput output); } diff --git a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll index 264698443bd..73ff6b256d9 100644 --- a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll @@ -24,7 +24,9 @@ predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) { private newtype TUnit = TMkUnit() -class Unit extends TUnit { +/** A singleton class containing a single dummy "unit" value. */ +private class Unit extends TUnit { + /** Gets a textual representation of this element. */ string toString() { result = "unit" } } @@ -106,6 +108,7 @@ predicate functionModelStep(FunctionModel fn, DataFlow::Node pred, DataFlow::Nod * a parameter or qualifier to a result. */ abstract class FunctionModel extends Function { + /** Holds if taint propagates through this function from `input` to `output`. */ abstract predicate hasTaintFlow(FunctionInput input, FunctionOutput output); } diff --git a/ql/src/semmle/go/security/ReflectedXssCustomizations.qll b/ql/src/semmle/go/security/ReflectedXssCustomizations.qll index bb7fc9ef0ca..37afda40110 100644 --- a/ql/src/semmle/go/security/ReflectedXssCustomizations.qll +++ b/ql/src/semmle/go/security/ReflectedXssCustomizations.qll @@ -40,14 +40,17 @@ module ReflectedXss { HttpResponseBodySink() { not nonHtmlContentType(this) } } - predicate htmlTypeSpecified(HTTP::ResponseBody body) { + /** + * Holds if `body` specifies the response's content type to be HTML. + */ + private predicate htmlTypeSpecified(HTTP::ResponseBody body) { exists(HTTP::HeaderWrite hw, string tp | hw = body.getResponseWriter().getAHeaderWrite() | hw.definesHeader("content-type", tp) and tp.regexpMatch("(?i).*html.*") ) } /** - * Holds if `h` may send a response with a content type other than HTML. + * Holds if `body` may send a response with a content type other than HTML. */ private predicate nonHtmlContentType(HTTP::ResponseBody body) { not htmlTypeSpecified(body) and diff --git a/ql/src/semmle/go/security/StringBreakCustomizations.qll b/ql/src/semmle/go/security/StringBreakCustomizations.qll index 29769cb0139..aa9ca293a2f 100644 --- a/ql/src/semmle/go/security/StringBreakCustomizations.qll +++ b/ql/src/semmle/go/security/StringBreakCustomizations.qll @@ -68,6 +68,7 @@ module StringBreak { override Quote getQuote() { result = quote } } + /** A call to `json.Unmarshal`, considered as a sanitizer for unsafe quoting. */ class UnmarshalSanitizer extends Sanitizer { UnmarshalSanitizer() { exists(Function jsonUnmarshal | jsonUnmarshal.hasQualifiedName("encoding/json", "Unmarshal") | @@ -76,6 +77,10 @@ module StringBreak { } } + /** + * A call to `strings.Replace` or `strings.ReplaceAll`, considered as a sanitizer + * for unsafe quoting. + */ class ReplaceSanitizer extends Sanitizer { Quote quote;