diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml deleted file mode 100644 index dcafb97dc98..00000000000 --- a/.github/workflows/labeler.yml +++ /dev/null @@ -1,11 +0,0 @@ -name: "Pull Request Labeler" -on: -- pull_request - -jobs: - triage: - runs-on: ubuntu-latest - steps: - - uses: actions/labeler@v2 - with: - repo-token: "${{ secrets.GITHUB_TOKEN }}" diff --git a/change-notes/1.24/analysis-javascript.md b/change-notes/1.24/analysis-javascript.md index c6c544e2f3d..f67bcbef676 100644 --- a/change-notes/1.24/analysis-javascript.md +++ b/change-notes/1.24/analysis-javascript.md @@ -27,4 +27,3 @@ ## Changes to libraries * The predicates `RegExpTerm.getSuccessor` and `RegExpTerm.getPredecessor` have been changed to reflect textual, not operational, matching order. This only makes a difference in lookbehind assertions, which are operationally matched backwards. Previously, `getSuccessor` would mimick this, so in an assertion `(?<=ab)` the term `b` would be considered the predecessor, not the successor, of `a`. Textually, however, `a` is still matched before `b`, and this is the order we now follow. - diff --git a/change-notes/1.24/extractor-javascript.md b/change-notes/1.24/extractor-javascript.md new file mode 100644 index 00000000000..0e8a3c978cf --- /dev/null +++ b/change-notes/1.24/extractor-javascript.md @@ -0,0 +1,7 @@ +[[ condition: enterprise-only ]] + +# Improvements to JavaScript analysis + +## Changes to code extraction + +* `import.meta` expressions no longer result in a syntax error in JavaScript files. diff --git a/config/identical-files.json b/config/identical-files.json index 29601d19510..b11f163b17b 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -265,5 +265,11 @@ "C# IR ValueNumberingImports": [ "csharp/ql/src/semmle/code/csharp/ir/implementation/raw/gvn/internal/ValueNumberingImports.qll", "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingImports.qll" + ], + "XML": [ + "cpp/ql/src/semmle/code/cpp/XML.qll", + "java/ql/src/semmle/code/xml/XML.qll", + "javascript/ql/src/semmle/javascript/XML.qll", + "python/ql/src/semmle/python/xml/XML.qll" ] } diff --git a/cpp/ql/src/AlertSuppression.ql b/cpp/ql/src/AlertSuppression.ql index 4fb22b8efbb..6258e8f7818 100644 --- a/cpp/ql/src/AlertSuppression.ql +++ b/cpp/ql/src/AlertSuppression.ql @@ -10,12 +10,25 @@ import cpp /** * An alert suppression comment. */ -class SuppressionComment extends CppStyleComment { +class SuppressionComment extends Comment { string annotation; string text; SuppressionComment() { - text = getContents().suffix(2) and + ( + this instanceof CppStyleComment and + // strip the beginning slashes + text = getContents().suffix(2) + or + this instanceof CStyleComment and + // strip both the beginning /* and the end */ the comment + exists(string text0 | + text0 = getContents().suffix(2) and + text = text0.prefix(text0.length() - 2) + ) and + // The /* */ comment must be a single-line comment + not text.matches("%\n%") + ) and ( // match `lgtm[...]` anywhere in the comment annotation = text.regexpFind("(?i)\\blgtm\\s*\\[[^\\]]*\\]", _, _) diff --git a/cpp/ql/src/semmle/code/cpp/NameQualifiers.qll b/cpp/ql/src/semmle/code/cpp/NameQualifiers.qll index f070e041765..eff2c9205bf 100644 --- a/cpp/ql/src/semmle/code/cpp/NameQualifiers.qll +++ b/cpp/ql/src/semmle/code/cpp/NameQualifiers.qll @@ -157,4 +157,6 @@ library class SpecialNameQualifyingElement extends NameQualifyingElement, @specialnamequalifyingelement { /** Gets the name of this special qualifying element. */ override string getName() { specialnamequalifyingelements(underlyingElement(this), result) } + + override string toString() { result = getName() } } diff --git a/cpp/ql/src/semmle/code/cpp/Preprocessor.qll b/cpp/ql/src/semmle/code/cpp/Preprocessor.qll index 7cf9aa60a24..7541a23b8bc 100644 --- a/cpp/ql/src/semmle/code/cpp/Preprocessor.qll +++ b/cpp/ql/src/semmle/code/cpp/Preprocessor.qll @@ -214,7 +214,9 @@ class PreprocessorUndef extends PreprocessorDirective, @ppd_undef { * A C/C++ preprocessor `#pragma` directive. */ class PreprocessorPragma extends PreprocessorDirective, @ppd_pragma { - override string toString() { result = "#pragma " + this.getHead() } + override string toString() { + if exists(this.getHead()) then result = "#pragma " + this.getHead() else result = "#pragma" + } } /** diff --git a/cpp/ql/src/semmle/code/cpp/XML.qll b/cpp/ql/src/semmle/code/cpp/XML.qll index 76e96fa1fc0..c6d55c8190b 100644 --- a/cpp/ql/src/semmle/code/cpp/XML.qll +++ b/cpp/ql/src/semmle/code/cpp/XML.qll @@ -2,7 +2,7 @@ * Provides classes and predicates for working with XML files and their content. */ -import semmle.code.cpp.Location +import semmle.files.FileSystem /** An XML element that has a location. */ abstract class XMLLocatable extends @xmllocatable { @@ -10,19 +10,22 @@ abstract class XMLLocatable extends @xmllocatable { Location getLocation() { xmllocations(this, result) } /** - * Holds if this element has the specified location information, - * including file path, start line, start column, end line and end column. + * Holds if this element is at the specified location. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. + * For more information, see + * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html). */ predicate hasLocationInfo( string filepath, int startline, int startcolumn, int endline, int endcolumn ) { exists(File f, Location l | l = this.getLocation() | - locations_default(l, unresolveElement(f), startline, startcolumn, endline, endcolumn) and + locations_default(l, f, startline, startcolumn, endline, endcolumn) and filepath = f.getAbsolutePath() ) } - /** Gets a printable representation of this element. */ + /** Gets a textual representation of this element. */ abstract string toString(); } @@ -31,6 +34,12 @@ abstract class XMLLocatable extends @xmllocatable { * both of which can contain other elements. */ class XMLParent extends @xmlparent { + XMLParent() { + // explicitly restrict `this` to be either an `XMLElement` or an `XMLFile`; + // the type `@xmlparent` currently also includes non-XML files + this instanceof @xmlelement or xmlEncoding(this, _) + } + /** * Gets a printable representation of this XML parent. * (Intended to be overridden in subclasses.) @@ -106,15 +115,21 @@ class XMLFile extends XMLParent, File { override string toString() { result = XMLParent.super.toString() } /** Gets the name of this XML file. */ - override string getName() { files(this, result, _, _, _) } + override string getName() { result = File.super.getAbsolutePath() } - /** Gets the path of this XML file. */ - string getPath() { files(this, _, result, _, _) } + /** + * DEPRECATED: Use `getAbsolutePath()` instead. + * + * Gets the path of this XML file. + */ + deprecated string getPath() { result = getAbsolutePath() } - /** Gets the path of the folder that contains this XML file. */ - string getFolder() { - result = this.getPath().substring(0, this.getPath().length() - this.getName().length()) - } + /** + * DEPRECATED: Use `getParentContainer().getAbsolutePath()` instead. + * + * Gets the path of the folder that contains this XML file. + */ + deprecated string getFolder() { result = getParentContainer().getAbsolutePath() } /** Gets the encoding of this XML file. */ string getEncoding() { xmlEncoding(this, result) } @@ -129,7 +144,17 @@ class XMLFile extends XMLParent, File { XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) } } -/** A "Document Type Definition" of an XML file. */ +/** + * An XML document type definition (DTD). + * + * Example: + * + * ``` + * + * + * + * ``` + */ class XMLDTD extends @xmldtd { /** Gets the name of the root element of this DTD. */ string getRoot() { xmlDTDs(this, result, _, _, _) } @@ -156,7 +181,17 @@ class XMLDTD extends @xmldtd { } } -/** An XML tag in an XML file. */ +/** + * An XML element in an XML file. + * + * Example: + * + * ``` + * + * + * ``` + */ class XMLElement extends @xmlelement, XMLParent, XMLLocatable { /** Holds if this XML element has the given `name`. */ predicate hasName(string name) { name = getName() } @@ -201,7 +236,16 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable { override string toString() { result = XMLParent.super.toString() } } -/** An attribute that occurs inside an XML element. */ +/** + * An attribute that occurs inside an XML element. + * + * Examples: + * + * ``` + * package="com.example.exampleapp" + * android:versionCode="1" + * ``` + */ class XMLAttribute extends @xmlattribute, XMLLocatable { /** Gets the name of this attribute. */ string getName() { xmlAttrs(this, _, result, _, _, _) } @@ -222,7 +266,15 @@ class XMLAttribute extends @xmlattribute, XMLLocatable { override string toString() { result = this.getName() + "=" + this.getValue() } } -/** A namespace used in an XML file */ +/** + * A namespace used in an XML file. + * + * Example: + * + * ``` + * xmlns:android="http://schemas.android.com/apk/res/android" + * ``` + */ class XMLNamespace extends @xmlnamespace { /** Gets the prefix of this namespace. */ string getPrefix() { xmlNs(this, result, _, _) } @@ -241,7 +293,15 @@ class XMLNamespace extends @xmlnamespace { } } -/** A comment of the form `` is an XML comment. */ +/** + * A comment in an XML file. + * + * Example: + * + * ``` + * + * ``` + */ class XMLComment extends @xmlcomment, XMLLocatable { /** Gets the text content of this XML comment. */ string getText() { xmlComments(this, result, _, _) } @@ -256,6 +316,12 @@ class XMLComment extends @xmlcomment, XMLLocatable { /** * A sequence of characters that occurs between opening and * closing tags of an XML element, excluding other elements. + * + * Example: + * + * ``` + * This is a sequence of characters. + * ``` */ class XMLCharacters extends @xmlcharacters, XMLLocatable { /** Gets the content of this character sequence. */ diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll index 6a7e2142efe..48a4446f3b5 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll @@ -19,10 +19,32 @@ private predicate predictableInstruction(Instruction instr) { predictableInstruction(instr.(UnaryInstruction).getUnary()) } +private predicate userInputInstruction(Instruction instr) { + exists(CallInstruction ci, WriteSideEffectInstruction wsei | + userInputArgument(ci.getConvertedResultExpression(), wsei.getIndex()) and + instr = wsei and + wsei.getPrimaryInstruction() = ci + ) + or + userInputReturned(instr.getConvertedResultExpression()) + or + instr.getConvertedResultExpression() instanceof EnvironmentRead + or + instr + .(LoadInstruction) + .getSourceAddress() + .(VariableAddressInstruction) + .getASTVariable() + .hasName("argv") and + instr.getEnclosingFunction().hasGlobalName("main") +} + private class DefaultTaintTrackingCfg extends DataFlow::Configuration { DefaultTaintTrackingCfg() { this = "DefaultTaintTrackingCfg" } - override predicate isSource(DataFlow::Node source) { isUserInput(source.asExpr(), _) } + override predicate isSource(DataFlow::Node source) { + userInputInstruction(source.asInstruction()) + } override predicate isSink(DataFlow::Node sink) { any() } @@ -135,6 +157,8 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) { // This is part of the translation of `a[i]`, where we want taint to flow // from `a`. i2.(PointerAddInstruction).getLeft() = i1 + // TODO: robust Chi handling + // // TODO: Flow from argument to return of known functions: Port missing parts // of `returnArgument` to the `interfaces.Taint` and `interfaces.DataFlow` // libraries. @@ -176,11 +200,30 @@ private Element adjustedSink(DataFlow::Node sink) { // For compatibility, send flow into a `NotExpr` even if it's part of a // short-circuiting condition and thus might get skipped. result.(NotExpr).getOperand() = sink.asExpr() + or + // For compatibility, send flow from argument read side effects to their + // corresponding argument expression + exists(IndirectReadSideEffectInstruction read | + read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and + read.getArgumentDef().getUnconvertedResultExpression() = result + ) + or + exists(BufferReadSideEffectInstruction read | + read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and + read.getArgumentDef().getUnconvertedResultExpression() = result + ) + or + exists(SizedBufferReadSideEffectInstruction read | + read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and + read.getArgumentDef().getUnconvertedResultExpression() = result + ) } predicate tainted(Expr source, Element tainted) { exists(DefaultTaintTrackingCfg cfg, DataFlow::Node sink | cfg.hasFlow(DataFlow::exprNode(source), sink) + or + cfg.hasFlow(DataFlow::definitionByReferenceNode(source), sink) | tainted = adjustedSink(sink) ) diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 24cfed86ba4..959b3da419b 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -173,11 +173,48 @@ abstract class PostUpdateNode extends Node { abstract Node getPreUpdateNode(); } +/** + * A node that represents the value of a variable after a function call that + * may have changed the variable because it's passed by reference. + * + * A typical example would be a call `f(&x)`. Firstly, there will be flow into + * `x` from previous definitions of `x`. Secondly, there will be a + * `DefinitionByReferenceNode` to represent the value of `x` after the call has + * returned. This node will have its `getArgument()` equal to `&x` and its + * `getVariableAccess()` equal to `x`. + */ +class DefinitionByReferenceNode extends Node { + override WriteSideEffectInstruction instr; + + /** Gets the argument corresponding to this node. */ + Expr getArgument() { + result = instr + .getPrimaryInstruction() + .(CallInstruction) + .getPositionalArgument(instr.getIndex()) + .getUnconvertedResultExpression() + or + result = instr + .getPrimaryInstruction() + .(CallInstruction) + .getThisArgument() + .getUnconvertedResultExpression() and + instr.getIndex() = -1 + } + + /** Gets the parameter through which this value is assigned. */ + Parameter getParameter() { + exists(CallInstruction ci | result = ci.getStaticCallTarget().getParameter(instr.getIndex())) + } +} + /** * Gets the node corresponding to `instr`. */ Node instructionNode(Instruction instr) { result.asInstruction() = instr } +DefinitionByReferenceNode definitionByReferenceNode(Expr e) { result.getArgument() = e } + /** * Gets a `Node` corresponding to `e` or any of its conversions. There is no * result if `e` is a `Conversion`. diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index 241de4406e8..469cd5bbd45 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction { IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing a side effect of a function call. */ -class WriteSideEffectInstruction extends SideEffectInstruction { +class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index 241de4406e8..469cd5bbd45 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction { IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing a side effect of a function call. */ -class WriteSideEffectInstruction extends SideEffectInstruction { +class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll index f0c006111f5..e27364810ef 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll @@ -143,26 +143,23 @@ private module Cached { .getInstructionSuccessor(getInstructionTag(instruction), kind) } - // This predicate has pragma[noopt] because otherwise the `getAChild*` calls - // get joined too early. The join order for the loop cases goes like this: - // - Find all loops of that type (tens of thousands). - // - Find all edges into the start of the loop (x 2). - // - Restrict to edges that originate within the loop (/ 2). - pragma[noopt] - cached - Instruction getInstructionBackEdgeSuccessor(Instruction instruction, EdgeKind kind) { + /** + * Holds if the CFG edge (`sourceElement`, `sourceTag`) ---`kind`--> + * `targetInstruction` is a back edge under the condition that + * `requiredAncestor` is an ancestor of `sourceElement`. + */ + private predicate backEdgeCandidate( + TranslatedElement sourceElement, InstructionTag sourceTag, TranslatedElement requiredAncestor, + Instruction targetInstruction, EdgeKind kind + ) { // While loop: // Any edge from within the body of the loop to the condition of the loop // is a back edge. This includes edges from `continue` and the fall-through // edge(s) after the last instruction(s) in the body. exists(TranslatedWhileStmt s | - s instanceof TranslatedWhileStmt and - result = s.getFirstConditionInstruction() and - exists(TranslatedElement inBody, InstructionTag tag | - result = inBody.getInstructionSuccessor(tag, kind) and - exists(TranslatedElement body | body = s.getBody() | inBody = body.getAChild*()) and - instruction = inBody.getInstruction(tag) - ) + targetInstruction = s.getFirstConditionInstruction() and + targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and + requiredAncestor = s.getBody() ) or // Do-while loop: @@ -171,15 +168,9 @@ private module Cached { // { ... } while (0)` statement. Note that all `continue` statements in a // do-while loop produce forward edges. exists(TranslatedDoStmt s | - s instanceof TranslatedDoStmt and - exists(TranslatedStmt body | body = s.getBody() | result = body.getFirstInstruction()) and - exists(TranslatedElement inCondition, InstructionTag tag | - result = inCondition.getInstructionSuccessor(tag, kind) and - exists(TranslatedElement condition | condition = s.getCondition() | - inCondition = condition.getAChild*() - ) and - instruction = inCondition.getInstruction(tag) - ) + targetInstruction = s.getBody().getFirstInstruction() and + targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and + requiredAncestor = s.getCondition() ) or // For loop: @@ -189,33 +180,42 @@ private module Cached { // last instruction(s) in the body. A for loop may not have a condition, in // which case `getFirstConditionInstruction` returns the body instead. exists(TranslatedForStmt s | - s instanceof TranslatedForStmt and - result = s.getFirstConditionInstruction() and - exists(TranslatedElement inLoop, InstructionTag tag | - result = inLoop.getInstructionSuccessor(tag, kind) and - exists(TranslatedElement bodyOrUpdate | - bodyOrUpdate = s.getBody() - or - bodyOrUpdate = s.getUpdate() - | - inLoop = bodyOrUpdate.getAChild*() - ) and - instruction = inLoop.getInstruction(tag) + targetInstruction = s.getFirstConditionInstruction() and + targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and + ( + requiredAncestor = s.getUpdate() + or + not exists(s.getUpdate()) and + requiredAncestor = s.getBody() ) ) or // Range-based for loop: // Any edge from within the update of the loop to the condition of // the loop is a back edge. - exists(TranslatedRangeBasedForStmt s, TranslatedCondition condition | - s instanceof TranslatedRangeBasedForStmt and - condition = s.getCondition() and - result = condition.getFirstInstruction() and - exists(TranslatedElement inUpdate, InstructionTag tag | - result = inUpdate.getInstructionSuccessor(tag, kind) and - exists(TranslatedElement update | update = s.getUpdate() | inUpdate = update.getAChild*()) and - instruction = inUpdate.getInstruction(tag) - ) + exists(TranslatedRangeBasedForStmt s | + targetInstruction = s.getCondition().getFirstInstruction() and + targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and + requiredAncestor = s.getUpdate() + ) + } + + private predicate jumpSourceHasAncestor(TranslatedElement jumpSource, TranslatedElement ancestor) { + backEdgeCandidate(jumpSource, _, _, _, _) and + ancestor = jumpSource + or + // For performance, we don't want a fastTC here + jumpSourceHasAncestor(jumpSource, ancestor.getAChild()) + } + + cached + Instruction getInstructionBackEdgeSuccessor(Instruction instruction, EdgeKind kind) { + exists( + TranslatedElement sourceElement, InstructionTag sourceTag, TranslatedElement requiredAncestor + | + backEdgeCandidate(sourceElement, sourceTag, requiredAncestor, result, kind) and + jumpSourceHasAncestor(sourceElement, requiredAncestor) and + instruction = sourceElement.getInstruction(sourceTag) ) or // Goto statement: @@ -225,7 +225,6 @@ private module Cached { // same location for source and target, so we conservatively assume that // such a `goto` creates a back edge. exists(TranslatedElement s, GotoStmt goto | - goto instanceof GotoStmt and not isStrictlyForwardGoto(goto) and goto = s.getAST() and exists(InstructionTag tag | diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index 241de4406e8..469cd5bbd45 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction { IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing a side effect of a function call. */ -class WriteSideEffectInstruction extends SideEffectInstruction { +class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } diff --git a/cpp/ql/src/semmle/code/cpp/models/Models.qll b/cpp/ql/src/semmle/code/cpp/models/Models.qll index 162b2f2f0c8..4f795bae1dd 100644 --- a/cpp/ql/src/semmle/code/cpp/models/Models.qll +++ b/cpp/ql/src/semmle/code/cpp/models/Models.qll @@ -1,5 +1,6 @@ private import implementations.Allocation private import implementations.Deallocation +private import implementations.Fread private import implementations.IdentityFunction private import implementations.Inet private import implementations.Memcpy diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll new file mode 100644 index 00000000000..8fdf17ead02 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll @@ -0,0 +1,14 @@ +import semmle.code.cpp.models.interfaces.Alias + +class Fread extends AliasFunction { + Fread() { this.hasGlobalName("fread") } + + override predicate parameterNeverEscapes(int n) { + n = 0 or + n = 3 + } + + override predicate parameterEscapesOnlyViaReturn(int n) { none() } + + override predicate parameterIsAlwaysReturned(int n) { none() } +} diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll index 58721897802..403e8fae1de 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll @@ -1,9 +1,10 @@ import semmle.code.cpp.models.interfaces.FormattingFunction +import semmle.code.cpp.models.interfaces.Alias /** * The standard functions `printf`, `wprintf` and their glib variants. */ -class Printf extends FormattingFunction { +class Printf extends FormattingFunction, AliasFunction { Printf() { this instanceof TopLevelFunction and ( @@ -22,6 +23,12 @@ class Printf extends FormattingFunction { hasGlobalOrStdName("wprintf") or hasGlobalName("wprintf_s") } + + override predicate parameterNeverEscapes(int n) { n = 0 } + + override predicate parameterEscapesOnlyViaReturn(int n) { none() } + + override predicate parameterIsAlwaysReturned(int n) { none() } } /** diff --git a/cpp/ql/test/library-tests/CPP-205/elements.expected b/cpp/ql/test/library-tests/CPP-205/elements.expected index fbfc97a4e46..4c0cc34cce7 100644 --- a/cpp/ql/test/library-tests/CPP-205/elements.expected +++ b/cpp/ql/test/library-tests/CPP-205/elements.expected @@ -26,6 +26,7 @@ | CPP-205.cpp:8:3:8:15 | return ... | | | CPP-205.cpp:8:10:8:11 | call to fn | | | CPP-205.cpp:8:13:8:13 | 0 | | +| file://:0:0:0:0 | __super | | | file://:0:0:0:0 | __va_list_tag | | | file://:0:0:0:0 | operator= | function __va_list_tag::operator=(__va_list_tag &&) -> __va_list_tag & | | file://:0:0:0:0 | operator= | function __va_list_tag::operator=(const __va_list_tag &) -> __va_list_tag & | diff --git a/cpp/ql/test/library-tests/clang_ms/element.expected b/cpp/ql/test/library-tests/clang_ms/element.expected index 6e6555e30b4..16241c2e057 100644 --- a/cpp/ql/test/library-tests/clang_ms/element.expected +++ b/cpp/ql/test/library-tests/clang_ms/element.expected @@ -23,6 +23,8 @@ | clang_ms.cpp:11:5:11:13 | asm statement | | clang_ms.cpp:12:1:12:1 | return ... | | clang_ms.cpp:16:1:16:19 | // Test for CPP-184 | +| clang_ms.cpp:17:1:17:32 | #pragma | +| clang_ms.cpp:18:1:18:31 | #pragma | | file://:0:0:0:0 | | | file://:0:0:0:0 | (global namespace) | | file://:0:0:0:0 | _Complex __float128 | @@ -48,6 +50,7 @@ | file://:0:0:0:0 | __ptr32 | | file://:0:0:0:0 | __ptr64 | | file://:0:0:0:0 | __sptr | +| file://:0:0:0:0 | __super | | file://:0:0:0:0 | __uptr | | file://:0:0:0:0 | __va_list_tag | | file://:0:0:0:0 | __va_list_tag & | diff --git a/cpp/ql/test/library-tests/conditions/elements.expected b/cpp/ql/test/library-tests/conditions/elements.expected index 32690d18d4b..4e94d425a8a 100644 --- a/cpp/ql/test/library-tests/conditions/elements.expected +++ b/cpp/ql/test/library-tests/conditions/elements.expected @@ -27,6 +27,7 @@ | file://:0:0:0:0 | __ptr32 | | file://:0:0:0:0 | __ptr64 | | file://:0:0:0:0 | __sptr | +| file://:0:0:0:0 | __super | | file://:0:0:0:0 | __uptr | | file://:0:0:0:0 | __va_list_tag | | file://:0:0:0:0 | __va_list_tag & | diff --git a/cpp/ql/test/library-tests/dataflow/fields/E.cpp b/cpp/ql/test/library-tests/dataflow/fields/E.cpp new file mode 100644 index 00000000000..fc745a9facc --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/fields/E.cpp @@ -0,0 +1,34 @@ +class buf +{ +public: + char *buffer; +}; + +class packet +{ +public: + buf data; +}; + +typedef long ssize_t; + +ssize_t argument_source(void *buf); + +void sink(char *b); + +void handlePacket(packet *p) +{ + sink(p->data.buffer); +} + +void f(buf* b) +{ + char *raw; + packet p; + argument_source(raw); + argument_source(b->buffer); + argument_source(p.data.buffer); + sink(raw); + sink(b->buffer); + handlePacket(&p); +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected index 3b928fe961d..ca851dc8974 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected +++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected @@ -122,6 +122,17 @@ edges | D.cpp:64:10:64:17 | boxfield [box, elem] | D.cpp:64:20:64:22 | box [elem] | | D.cpp:64:10:64:17 | this [boxfield, box, ... (3)] | D.cpp:64:10:64:17 | boxfield [box, elem] | | D.cpp:64:20:64:22 | box [elem] | D.cpp:64:25:64:28 | elem | +| E.cpp:19:27:19:27 | p [data, buffer] | E.cpp:21:10:21:10 | p [data, buffer] | +| E.cpp:21:10:21:10 | p [data, buffer] | E.cpp:21:13:21:16 | data [buffer] | +| E.cpp:21:13:21:16 | data [buffer] | E.cpp:21:18:21:23 | buffer | +| E.cpp:28:21:28:23 | ref arg raw | E.cpp:31:10:31:12 | raw | +| E.cpp:29:21:29:21 | b [post update] [buffer] | E.cpp:32:10:32:10 | b [buffer] | +| E.cpp:29:24:29:29 | ref arg buffer | E.cpp:29:21:29:21 | b [post update] [buffer] | +| E.cpp:30:21:30:21 | p [post update] [data, buffer] | E.cpp:33:18:33:19 | & ... [data, buffer] | +| E.cpp:30:23:30:26 | data [post update] [buffer] | E.cpp:30:21:30:21 | p [post update] [data, buffer] | +| E.cpp:30:28:30:33 | ref arg buffer | E.cpp:30:23:30:26 | data [post update] [buffer] | +| E.cpp:32:10:32:10 | b [buffer] | E.cpp:32:13:32:18 | buffer | +| E.cpp:33:18:33:19 | & ... [data, buffer] | E.cpp:19:27:19:27 | p [data, buffer] | | aliasing.cpp:9:3:9:3 | s [post update] [m1] | aliasing.cpp:25:17:25:19 | ref arg & ... [m1] | | aliasing.cpp:9:3:9:22 | ... = ... | aliasing.cpp:9:3:9:3 | s [post update] [m1] | | aliasing.cpp:9:11:9:20 | call to user_input | aliasing.cpp:9:3:9:22 | ... = ... | @@ -378,6 +389,20 @@ nodes | D.cpp:64:10:64:17 | this [boxfield, box, ... (3)] | semmle.label | this [boxfield, box, ... (3)] | | D.cpp:64:20:64:22 | box [elem] | semmle.label | box [elem] | | D.cpp:64:25:64:28 | elem | semmle.label | elem | +| E.cpp:19:27:19:27 | p [data, buffer] | semmle.label | p [data, buffer] | +| E.cpp:21:10:21:10 | p [data, buffer] | semmle.label | p [data, buffer] | +| E.cpp:21:13:21:16 | data [buffer] | semmle.label | data [buffer] | +| E.cpp:21:18:21:23 | buffer | semmle.label | buffer | +| E.cpp:28:21:28:23 | ref arg raw | semmle.label | ref arg raw | +| E.cpp:29:21:29:21 | b [post update] [buffer] | semmle.label | b [post update] [buffer] | +| E.cpp:29:24:29:29 | ref arg buffer | semmle.label | ref arg buffer | +| E.cpp:30:21:30:21 | p [post update] [data, buffer] | semmle.label | p [post update] [data, buffer] | +| E.cpp:30:23:30:26 | data [post update] [buffer] | semmle.label | data [post update] [buffer] | +| E.cpp:30:28:30:33 | ref arg buffer | semmle.label | ref arg buffer | +| E.cpp:31:10:31:12 | raw | semmle.label | raw | +| E.cpp:32:10:32:10 | b [buffer] | semmle.label | b [buffer] | +| E.cpp:32:13:32:18 | buffer | semmle.label | buffer | +| E.cpp:33:18:33:19 | & ... [data, buffer] | semmle.label | & ... [data, buffer] | | aliasing.cpp:9:3:9:3 | s [post update] [m1] | semmle.label | s [post update] [m1] | | aliasing.cpp:9:3:9:22 | ... = ... | semmle.label | ... = ... | | aliasing.cpp:9:11:9:20 | call to user_input | semmle.label | call to user_input | @@ -532,6 +557,9 @@ nodes | D.cpp:22:25:22:31 | call to getElem | D.cpp:42:15:42:24 | new | D.cpp:22:25:22:31 | call to getElem | call to getElem flows from $@ | D.cpp:42:15:42:24 | new | new | | D.cpp:22:25:22:31 | call to getElem | D.cpp:49:15:49:24 | new | D.cpp:22:25:22:31 | call to getElem | call to getElem flows from $@ | D.cpp:49:15:49:24 | new | new | | D.cpp:64:25:64:28 | elem | D.cpp:56:15:56:24 | new | D.cpp:64:25:64:28 | elem | elem flows from $@ | D.cpp:56:15:56:24 | new | new | +| E.cpp:21:18:21:23 | buffer | E.cpp:30:28:30:33 | ref arg buffer | E.cpp:21:18:21:23 | buffer | buffer flows from $@ | E.cpp:30:28:30:33 | ref arg buffer | ref arg buffer | +| E.cpp:31:10:31:12 | raw | E.cpp:28:21:28:23 | ref arg raw | E.cpp:31:10:31:12 | raw | raw flows from $@ | E.cpp:28:21:28:23 | ref arg raw | ref arg raw | +| E.cpp:32:13:32:18 | buffer | E.cpp:29:24:29:29 | ref arg buffer | E.cpp:32:13:32:18 | buffer | buffer flows from $@ | E.cpp:29:24:29:29 | ref arg buffer | ref arg buffer | | aliasing.cpp:29:11:29:12 | m1 | aliasing.cpp:9:11:9:20 | call to user_input | aliasing.cpp:29:11:29:12 | m1 | m1 flows from $@ | aliasing.cpp:9:11:9:20 | call to user_input | call to user_input | | aliasing.cpp:30:11:30:12 | m1 | aliasing.cpp:13:10:13:19 | call to user_input | aliasing.cpp:30:11:30:12 | m1 | m1 flows from $@ | aliasing.cpp:13:10:13:19 | call to user_input | call to user_input | | aliasing.cpp:62:14:62:15 | m1 | aliasing.cpp:60:11:60:20 | call to user_input | aliasing.cpp:62:14:62:15 | m1 | m1 flows from $@ | aliasing.cpp:60:11:60:20 | call to user_input | call to user_input | diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.ql b/cpp/ql/test/library-tests/dataflow/fields/flow.ql index 131973534bf..b1ccc100d39 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/flow.ql +++ b/cpp/ql/test/library-tests/dataflow/fields/flow.ql @@ -14,6 +14,11 @@ class Conf extends Configuration { src.asExpr() instanceof NewExpr or src.asExpr().(Call).getTarget().hasName("user_input") + or + exists(FunctionCall fc | + fc.getAnArgument() = src.asDefiningArgument() and + fc.getTarget().hasName("argument_source") + ) } override predicate isSink(Node sink) { diff --git a/cpp/ql/test/library-tests/files/FilePath.expected b/cpp/ql/test/library-tests/files/FilePath.expected deleted file mode 100644 index 6647c1e1fd7..00000000000 --- a/cpp/ql/test/library-tests/files/FilePath.expected +++ /dev/null @@ -1,4 +0,0 @@ -| c.c | library-tests/files/c.c | -| files1.cpp | library-tests/files/files1.cpp | -| files1.h | library-tests/files/files1.h | -| files2.cpp | library-tests/files/files2.cpp | diff --git a/cpp/ql/test/library-tests/files/FilePath.ql b/cpp/ql/test/library-tests/files/FilePath.ql deleted file mode 100644 index 724c4b45ae7..00000000000 --- a/cpp/ql/test/library-tests/files/FilePath.ql +++ /dev/null @@ -1,5 +0,0 @@ -import cpp - -from File f -where f.toString() != "" -select f.toString(), f.getRelativePath() diff --git a/cpp/ql/test/library-tests/files/Files.expected b/cpp/ql/test/library-tests/files/Files.expected new file mode 100644 index 00000000000..68187793433 --- /dev/null +++ b/cpp/ql/test/library-tests/files/Files.expected @@ -0,0 +1,4 @@ +| c.c | library-tests/files/c.c | CFile, MetricFile | C | | | +| files1.cpp | library-tests/files/files1.cpp | CppFile, MetricFile | C++ | swap | t | +| files1.h | library-tests/files/files1.h | HeaderFile, MetricFile | | swap | | +| files2.cpp | library-tests/files/files2.cpp | CppFile, MetricFile | C++ | g | x, y | diff --git a/cpp/ql/test/library-tests/files/Files.ql b/cpp/ql/test/library-tests/files/Files.ql new file mode 100644 index 00000000000..ee489c01843 --- /dev/null +++ b/cpp/ql/test/library-tests/files/Files.ql @@ -0,0 +1,18 @@ +import cpp + +string describe(File f) { + f.compiledAsC() and + result = "C" + or + f.compiledAsCpp() and + result = "C++" + or + f instanceof XMLParent and + result = "XMLParent" // regression tests a bug in the characteristic predicate of XMLParent +} + +from File f +where f.toString() != "" +select f.toString(), f.getRelativePath(), concat(f.getAQlClass().toString(), ", "), + concat(describe(f), ", "), concat(f.getATopLevelDeclaration().toString(), ", "), + concat(LocalVariable v | f.getADeclaration() = v | v.toString(), ", ") diff --git a/cpp/ql/test/library-tests/files/Files1.expected b/cpp/ql/test/library-tests/files/Files1.expected deleted file mode 100644 index ef11fb83e9b..00000000000 --- a/cpp/ql/test/library-tests/files/Files1.expected +++ /dev/null @@ -1 +0,0 @@ -| files1.h:0:0:0:0 | files1.h | files1.cpp:4:6:4:9 | swap | diff --git a/cpp/ql/test/library-tests/files/Files1.ql b/cpp/ql/test/library-tests/files/Files1.ql deleted file mode 100644 index a4db35de1b8..00000000000 --- a/cpp/ql/test/library-tests/files/Files1.ql +++ /dev/null @@ -1,4 +0,0 @@ -import cpp - -from HeaderFile f -select f, f.getATopLevelDeclaration() diff --git a/cpp/ql/test/library-tests/files/Files2.expected b/cpp/ql/test/library-tests/files/Files2.expected deleted file mode 100644 index 7a2649c09e4..00000000000 --- a/cpp/ql/test/library-tests/files/Files2.expected +++ /dev/null @@ -1,8 +0,0 @@ -| CFile | C | --- | c.c | -| CppFile | - | C++ | files1.cpp | -| CppFile | - | C++ | files2.cpp | -| HeaderFile | - | --- | files1.h | -| MetricFile | - | --- | files1.h | -| MetricFile | - | C++ | files1.cpp | -| MetricFile | - | C++ | files2.cpp | -| MetricFile | C | --- | c.c | diff --git a/cpp/ql/test/library-tests/files/Files2.ql b/cpp/ql/test/library-tests/files/Files2.ql deleted file mode 100644 index b87a06b9761..00000000000 --- a/cpp/ql/test/library-tests/files/Files2.ql +++ /dev/null @@ -1,11 +0,0 @@ -import cpp - -string isCompiledAsC(File f) { if f.compiledAsC() then result = "C" else result = "-" } - -string isCompiledAsCpp(File f) { if f.compiledAsCpp() then result = "C++" else result = "---" } - -from File f -// On 64bit Linux, __va_list_tag is in the unknown file (""). Ignore it. -where f.getAbsolutePath() != "" -select (f.getAQlClass().toString() + " ").prefix(10), isCompiledAsC(f), isCompiledAsCpp(f), - f.toString() diff --git a/cpp/ql/test/library-tests/files/Files3.expected b/cpp/ql/test/library-tests/files/Files3.expected deleted file mode 100644 index f17f4192871..00000000000 --- a/cpp/ql/test/library-tests/files/Files3.expected +++ /dev/null @@ -1,3 +0,0 @@ -| files1.cpp | files1.cpp:4:6:4:9 | swap | -| files1.h | files1.cpp:4:6:4:9 | swap | -| files2.cpp | files2.cpp:3:6:3:6 | g | diff --git a/cpp/ql/test/library-tests/files/Files3.ql b/cpp/ql/test/library-tests/files/Files3.ql deleted file mode 100644 index 9d8d2ffc83c..00000000000 --- a/cpp/ql/test/library-tests/files/Files3.ql +++ /dev/null @@ -1,7 +0,0 @@ -import cpp - -from File f, Declaration d -where - d = f.getATopLevelDeclaration() and - d.getName() != "__va_list_tag" -select f.toString(), d diff --git a/cpp/ql/test/library-tests/files/Files4.expected b/cpp/ql/test/library-tests/files/Files4.expected deleted file mode 100644 index 188b7161d99..00000000000 --- a/cpp/ql/test/library-tests/files/Files4.expected +++ /dev/null @@ -1,3 +0,0 @@ -| files1.cpp | files1.cpp:6:6:6:6 | t | -| files2.cpp | files2.cpp:4:6:4:6 | x | -| files2.cpp | files2.cpp:5:6:5:6 | y | diff --git a/cpp/ql/test/library-tests/files/Files4.ql b/cpp/ql/test/library-tests/files/Files4.ql deleted file mode 100644 index 10402979541..00000000000 --- a/cpp/ql/test/library-tests/files/Files4.ql +++ /dev/null @@ -1,5 +0,0 @@ -import cpp - -from File f, LocalVariable v -where f.getADeclaration() = v -select f.toString(), v diff --git a/cpp/ql/test/library-tests/lambdas/captures/elements.expected b/cpp/ql/test/library-tests/lambdas/captures/elements.expected index ba4e26e7d8f..598dec39fe5 100644 --- a/cpp/ql/test/library-tests/lambdas/captures/elements.expected +++ b/cpp/ql/test/library-tests/lambdas/captures/elements.expected @@ -201,6 +201,7 @@ | file://:0:0:0:0 | ..(*)(..) | | file://:0:0:0:0 | ..(*)(..) | | file://:0:0:0:0 | ..(..) | +| file://:0:0:0:0 | __super | | file://:0:0:0:0 | __va_list_tag | | file://:0:0:0:0 | __va_list_tag & | | file://:0:0:0:0 | __va_list_tag && | diff --git a/cpp/ql/test/library-tests/macros/inmacroexpansion/inmacroexpansion.expected b/cpp/ql/test/library-tests/macros/inmacroexpansion/inmacroexpansion.expected index 580e3da8356..b60d112c917 100644 --- a/cpp/ql/test/library-tests/macros/inmacroexpansion/inmacroexpansion.expected +++ b/cpp/ql/test/library-tests/macros/inmacroexpansion/inmacroexpansion.expected @@ -1,3 +1,4 @@ +| file://:0:0:0:0 | __super | false | | file://:0:0:0:0 | __va_list_tag | false | | file://:0:0:0:0 | operator= | false | | file://:0:0:0:0 | operator= | false | diff --git a/cpp/ql/test/library-tests/ptr_to_member/segfault/exprs.ql b/cpp/ql/test/library-tests/ptr_to_member/segfault/exprs.ql index 530939c66c1..7dae27315c9 100644 --- a/cpp/ql/test/library-tests/ptr_to_member/segfault/exprs.ql +++ b/cpp/ql/test/library-tests/ptr_to_member/segfault/exprs.ql @@ -1,4 +1,5 @@ import cpp from Expr e +where exists(e.toString()) select e, e.getType() diff --git a/cpp/ql/test/library-tests/templates/CPP-204/element.expected b/cpp/ql/test/library-tests/templates/CPP-204/element.expected index 44384471af9..f04fcc8d7ba 100644 --- a/cpp/ql/test/library-tests/templates/CPP-204/element.expected +++ b/cpp/ql/test/library-tests/templates/CPP-204/element.expected @@ -7,6 +7,7 @@ | file://:0:0:0:0 | X | | file://:0:0:0:0 | X | | file://:0:0:0:0 | Y | +| file://:0:0:0:0 | __super | | file://:0:0:0:0 | __va_list_tag | | file://:0:0:0:0 | __va_list_tag & | | file://:0:0:0:0 | __va_list_tag && | diff --git a/cpp/ql/test/library-tests/templates/instantiations_functions/elements.expected b/cpp/ql/test/library-tests/templates/instantiations_functions/elements.expected index 99a6201c59c..df99979ff44 100644 --- a/cpp/ql/test/library-tests/templates/instantiations_functions/elements.expected +++ b/cpp/ql/test/library-tests/templates/instantiations_functions/elements.expected @@ -32,6 +32,7 @@ | file://:0:0:0:0 | __ptr32 | | file://:0:0:0:0 | __ptr64 | | file://:0:0:0:0 | __sptr | +| file://:0:0:0:0 | __super | | file://:0:0:0:0 | __uptr | | file://:0:0:0:0 | __va_list_tag | | file://:0:0:0:0 | __va_list_tag & | diff --git a/cpp/ql/test/library-tests/unnamed/elements.expected b/cpp/ql/test/library-tests/unnamed/elements.expected index a81f2550d5b..936ffa7575a 100644 --- a/cpp/ql/test/library-tests/unnamed/elements.expected +++ b/cpp/ql/test/library-tests/unnamed/elements.expected @@ -24,6 +24,7 @@ | file://:0:0:0:0 | __ptr32 | Other | | file://:0:0:0:0 | __ptr64 | Other | | file://:0:0:0:0 | __sptr | Other | +| file://:0:0:0:0 | __super | Other | | file://:0:0:0:0 | __uptr | Other | | file://:0:0:0:0 | __va_list_tag | Other | | file://:0:0:0:0 | abstract | Other | diff --git a/cpp/ql/test/library-tests/unnamed/elements.ql b/cpp/ql/test/library-tests/unnamed/elements.ql index 84c42736930..16a025063b9 100644 --- a/cpp/ql/test/library-tests/unnamed/elements.ql +++ b/cpp/ql/test/library-tests/unnamed/elements.ql @@ -3,5 +3,6 @@ import cpp from Element e, string s where not e instanceof Folder and + exists(e.toString()) and // Work around `VariableDeclarationEntry.toString()` not holding if e instanceof VariableAccess then s = "Variable access" else s = "Other" select e, s diff --git a/cpp/ql/test/query-tests/AlertSuppression/AlertSuppression.expected b/cpp/ql/test/query-tests/AlertSuppression/AlertSuppression.expected index a49930b2a10..3236b7f6183 100644 --- a/cpp/ql/test/query-tests/AlertSuppression/AlertSuppression.expected +++ b/cpp/ql/test/query-tests/AlertSuppression/AlertSuppression.expected @@ -8,6 +8,7 @@ | tst.c:8:1:8:18 | // lgtm: blah blah | lgtm: blah blah | lgtm | tst.c:8:1:8:18 | // lgtm: blah blah | | tst.c:9:1:9:32 | // lgtm blah blah #falsepositive | lgtm blah blah #falsepositive | lgtm | tst.c:9:1:9:32 | // lgtm blah blah #falsepositive | | tst.c:10:1:10:39 | //lgtm [js/invocation-of-non-function] | lgtm [js/invocation-of-non-function] | lgtm [js/invocation-of-non-function] | tst.c:10:1:10:39 | //lgtm [js/invocation-of-non-function] | +| tst.c:11:1:11:10 | /* lgtm */ | lgtm | lgtm | tst.c:11:1:11:10 | /* lgtm */ | | tst.c:12:1:12:9 | // lgtm[] | lgtm[] | lgtm[] | tst.c:12:1:12:9 | // lgtm[] | | tst.c:14:1:14:6 | //lgtm | lgtm | lgtm | tst.c:14:1:14:6 | //lgtm | | tst.c:15:1:15:7 | //\tlgtm | \tlgtm | lgtm | tst.c:15:1:15:7 | //\tlgtm | @@ -22,6 +23,10 @@ | tst.c:27:1:27:70 | // lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] | lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] | lgtm[js/invocation-of-non-function] | tst.c:27:1:27:70 | // lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] | | tst.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | lgtm[js/debugger-statement]; lgtm | lgtm | tst.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | | tst.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | lgtm[js/debugger-statement]; lgtm | lgtm[js/debugger-statement] | tst.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | +| tst.c:29:1:29:12 | /* lgtm[] */ | lgtm[] | lgtm[] | tst.c:29:1:29:12 | /* lgtm[] */ | +| tst.c:30:1:30:41 | /* lgtm[js/invocation-of-non-function] */ | lgtm[js/invocation-of-non-function] | lgtm[js/invocation-of-non-function] | tst.c:30:1:30:41 | /* lgtm[js/invocation-of-non-function] */ | +| tst.c:36:1:36:55 | /* lgtm[@tag:nullness,js/invocation-of-non-function] */ | lgtm[@tag:nullness,js/invocation-of-non-function] | lgtm[@tag:nullness,js/invocation-of-non-function] | tst.c:36:1:36:55 | /* lgtm[@tag:nullness,js/invocation-of-non-function] */ | +| tst.c:37:1:37:25 | /* lgtm[@tag:nullness] */ | lgtm[@tag:nullness] | lgtm[@tag:nullness] | tst.c:37:1:37:25 | /* lgtm[@tag:nullness] */ | | tstWindows.c:1:12:1:18 | // lgtm | lgtm | lgtm | tstWindows.c:1:1:1:18 | // lgtm | | tstWindows.c:2:1:2:30 | // lgtm[js/debugger-statement] | lgtm[js/debugger-statement] | lgtm[js/debugger-statement] | tstWindows.c:2:1:2:30 | // lgtm[js/debugger-statement] | | tstWindows.c:3:1:3:61 | // lgtm[js/debugger-statement, js/invocation-of-non-function] | lgtm[js/debugger-statement, js/invocation-of-non-function] | lgtm[js/debugger-statement, js/invocation-of-non-function] | tstWindows.c:3:1:3:61 | // lgtm[js/debugger-statement, js/invocation-of-non-function] | @@ -32,6 +37,7 @@ | tstWindows.c:8:1:8:18 | // lgtm: blah blah | lgtm: blah blah | lgtm | tstWindows.c:8:1:8:18 | // lgtm: blah blah | | tstWindows.c:9:1:9:32 | // lgtm blah blah #falsepositive | lgtm blah blah #falsepositive | lgtm | tstWindows.c:9:1:9:32 | // lgtm blah blah #falsepositive | | tstWindows.c:10:1:10:39 | //lgtm [js/invocation-of-non-function] | lgtm [js/invocation-of-non-function] | lgtm [js/invocation-of-non-function] | tstWindows.c:10:1:10:39 | //lgtm [js/invocation-of-non-function] | +| tstWindows.c:11:1:11:10 | /* lgtm */ | lgtm | lgtm | tstWindows.c:11:1:11:10 | /* lgtm */ | | tstWindows.c:12:1:12:9 | // lgtm[] | lgtm[] | lgtm[] | tstWindows.c:12:1:12:9 | // lgtm[] | | tstWindows.c:14:1:14:6 | //lgtm | lgtm | lgtm | tstWindows.c:14:1:14:6 | //lgtm | | tstWindows.c:15:1:15:7 | //\tlgtm | \tlgtm | lgtm | tstWindows.c:15:1:15:7 | //\tlgtm | @@ -46,3 +52,7 @@ | tstWindows.c:27:1:27:70 | // lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] | lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] | lgtm[js/invocation-of-non-function] | tstWindows.c:27:1:27:70 | // lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] | | tstWindows.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | lgtm[js/debugger-statement]; lgtm | lgtm | tstWindows.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | | tstWindows.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | lgtm[js/debugger-statement]; lgtm | lgtm[js/debugger-statement] | tstWindows.c:28:1:28:36 | // lgtm[js/debugger-statement]; lgtm | +| tstWindows.c:29:1:29:12 | /* lgtm[] */ | lgtm[] | lgtm[] | tstWindows.c:29:1:29:12 | /* lgtm[] */ | +| tstWindows.c:30:1:30:41 | /* lgtm[js/invocation-of-non-function] */ | lgtm[js/invocation-of-non-function] | lgtm[js/invocation-of-non-function] | tstWindows.c:30:1:30:41 | /* lgtm[js/invocation-of-non-function] */ | +| tstWindows.c:36:1:36:55 | /* lgtm[@tag:nullness,js/invocation-of-non-function] */ | lgtm[@tag:nullness,js/invocation-of-non-function] | lgtm[@tag:nullness,js/invocation-of-non-function] | tstWindows.c:36:1:36:55 | /* lgtm[@tag:nullness,js/invocation-of-non-function] */ | +| tstWindows.c:37:1:37:25 | /* lgtm[@tag:nullness] */ | lgtm[@tag:nullness] | lgtm[@tag:nullness] | tstWindows.c:37:1:37:25 | /* lgtm[@tag:nullness] */ | diff --git a/cpp/ql/test/query-tests/AlertSuppression/tst.c b/cpp/ql/test/query-tests/AlertSuppression/tst.c index a669e765981..4ca26b4f6eb 100644 --- a/cpp/ql/test/query-tests/AlertSuppression/tst.c +++ b/cpp/ql/test/query-tests/AlertSuppression/tst.c @@ -26,3 +26,12 @@ int x = 0; // lgtm // LGTM[js/debugger-statement] // lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] // lgtm[js/debugger-statement]; lgtm +/* lgtm[] */ +/* lgtm[js/invocation-of-non-function] */ +/* lgtm +*/ +/* lgtm + +*/ +/* lgtm[@tag:nullness,js/invocation-of-non-function] */ +/* lgtm[@tag:nullness] */ \ No newline at end of file diff --git a/cpp/ql/test/query-tests/AlertSuppression/tstWindows.c b/cpp/ql/test/query-tests/AlertSuppression/tstWindows.c index a669e765981..4ca26b4f6eb 100644 --- a/cpp/ql/test/query-tests/AlertSuppression/tstWindows.c +++ b/cpp/ql/test/query-tests/AlertSuppression/tstWindows.c @@ -26,3 +26,12 @@ int x = 0; // lgtm // LGTM[js/debugger-statement] // lgtm[js/debugger-statement] and lgtm[js/invocation-of-non-function] // lgtm[js/debugger-statement]; lgtm +/* lgtm[] */ +/* lgtm[js/invocation-of-non-function] */ +/* lgtm +*/ +/* lgtm + +*/ +/* lgtm[@tag:nullness,js/invocation-of-non-function] */ +/* lgtm[@tag:nullness] */ \ No newline at end of file diff --git a/csharp/ql/src/semmle/code/csharp/XML.qll b/csharp/ql/src/semmle/code/csharp/XML.qll index db84dc35baf..d5399e27740 100644 --- a/csharp/ql/src/semmle/code/csharp/XML.qll +++ b/csharp/ql/src/semmle/code/csharp/XML.qll @@ -5,13 +5,6 @@ import semmle.code.csharp.Element import semmle.code.csharp.Location -/** Adapter so that XMLLocatables are elements */ -library class XMLLocatableElement extends @xmllocatable, Element { - override string toString() { result = this.(XMLLocatable).toString() } - - override Location getALocation() { result = this.(XMLLocatable).getALocation() } -} - /** An XML element that has a location. */ class XMLLocatable extends @xmllocatable { XMLLocatable() { diff --git a/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll b/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll index 00841dbe3ce..3841a96d9f3 100644 --- a/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll +++ b/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll @@ -793,7 +793,10 @@ module Internal { TBooleanValue(boolean b) { b = true or b = false } or TIntegerValue(int i) { i = any(Expr e).getValue().toInt() } or TNullValue(boolean b) { b = true or b = false } or - TMatchValue(Case c, boolean b) { b = true or b = false } or + TMatchValue(Case c, boolean b) { + exists(c.getPattern()) and + (b = true or b = false) + } or TEmptyCollectionValue(boolean b) { b = true or b = false } /** A callable that always returns a `null` value. */ diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index 912e9ec1280..06a82f8ef4d 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -355,7 +355,7 @@ private module Cached { TSsaDefinitionNode(Ssa::Definition def) or TInstanceParameterNode(Callable c) { c.hasBody() and not c.(Modifiable).isStatic() } or TCilParameterNode(CIL::Parameter p) { p.getMethod().hasBody() } or - TTaintedParameterNode(Parameter p) { p.getCallable().hasBody() } or + TTaintedParameterNode(Parameter p) { explicitParameterNode(_, p) } or TTaintedReturnNode(ControlFlow::Nodes::ElementNode cfn) { any(Callable c).canYieldReturn(cfn.getElement()) } or diff --git a/csharp/ql/src/semmle/code/csharp/exprs/Call.qll b/csharp/ql/src/semmle/code/csharp/exprs/Call.qll index 5eba096e8b4..9e451781c54 100644 --- a/csharp/ql/src/semmle/code/csharp/exprs/Call.qll +++ b/csharp/ql/src/semmle/code/csharp/exprs/Call.qll @@ -295,7 +295,7 @@ class MethodCall extends Call, QualifiableExpr, LateBindableExpr, @method_invoca override Method getQualifiedDeclaration() { result = getTarget() } - override string toString() { result = "call to method " + this.getTarget().getName() } + override string toString() { result = "call to method " + concat(this.getTarget().getName()) } override Expr getRawArgument(int i) { if exists(getQualifier()) diff --git a/csharp/ql/src/semmle/code/csharp/exprs/Expr.qll b/csharp/ql/src/semmle/code/csharp/exprs/Expr.qll index 6fa563b99ef..e3226db31cb 100644 --- a/csharp/ql/src/semmle/code/csharp/exprs/Expr.qll +++ b/csharp/ql/src/semmle/code/csharp/exprs/Expr.qll @@ -116,10 +116,17 @@ private predicate isDynamicElementAccess(@dynamic_element_access_expr e) { any() * A local variable declaration, for example `var i = 0`. */ class LocalVariableDeclExpr extends Expr, @local_var_decl_expr { - /** Gets the local variable being declared. */ + /** + * Gets the local variable being declared, if any. The only case where + * no variable is declared is when a discard symbol is used, for example + * ``` + * if (int.TryParse(s, out var _)) + * ... + * ``` + */ LocalVariable getVariable() { localvars(result, _, _, _, _, this) } - /** Gets the name of the variable being declared. */ + /** Gets the name of the variable being declared, if any. */ string getName() { result = this.getVariable().getName() } /** Gets the initializer expression of this local variable declaration, if any. */ @@ -136,6 +143,9 @@ class LocalVariableDeclExpr extends Expr, @local_var_decl_expr { override string toString() { result = this.getVariable().getType().getName() + " " + this.getName() + or + not exists(this.getVariable()) and + result = "_" } /** Gets the variable access used in this declaration, if any. */ diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll index 241de4406e8..469cd5bbd45 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll @@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction { IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing a side effect of a function call. */ -class WriteSideEffectInstruction extends SideEffectInstruction { +class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll index 241de4406e8..469cd5bbd45 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll @@ -1236,6 +1236,8 @@ class IndirectReadSideEffectInstruction extends SideEffectInstruction { IndirectReadSideEffectInstruction() { getOpcode() instanceof Opcode::IndirectReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1245,6 +1247,8 @@ class BufferReadSideEffectInstruction extends SideEffectInstruction { BufferReadSideEffectInstruction() { getOpcode() instanceof Opcode::BufferReadSideEffect } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** @@ -1258,12 +1262,14 @@ class SizedBufferReadSideEffectInstruction extends SideEffectInstruction { Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } + + Instruction getSideEffect() { result = getAnOperand().(SideEffectOperand).getDef() } } /** * An instruction representing a side effect of a function call. */ -class WriteSideEffectInstruction extends SideEffectInstruction { +class WriteSideEffectInstruction extends SideEffectInstruction, IndexedInstruction { WriteSideEffectInstruction() { getOpcode() instanceof WriteSideEffectOpcode } Instruction getArgumentDef() { result = getAnOperand().(AddressOperand).getDef() } diff --git a/csharp/ql/test/library-tests/dataflow/global/GetAnOutNode.expected b/csharp/ql/test/library-tests/dataflow/global/GetAnOutNode.expected index 34b473e5659..586a0ef9ca8 100644 --- a/csharp/ql/test/library-tests/dataflow/global/GetAnOutNode.expected +++ b/csharp/ql/test/library-tests/dataflow/global/GetAnOutNode.expected @@ -82,8 +82,8 @@ | GlobalDataFlow.cs:100:24:100:33 | call to method Return | return | GlobalDataFlow.cs:100:24:100:33 | call to method Return | | GlobalDataFlow.cs:102:28:102:63 | call to method GetMethod | return | GlobalDataFlow.cs:102:28:102:63 | call to method GetMethod | | GlobalDataFlow.cs:102:28:102:103 | call to method Invoke | return | GlobalDataFlow.cs:102:28:102:103 | call to method Invoke | -| GlobalDataFlow.cs:104:9:104:46 | call to method ReturnOut | out | GlobalDataFlow.cs:104:27:104:34 | SSA def(nonSink0) | -| GlobalDataFlow.cs:104:9:104:46 | call to method ReturnOut | ref | GlobalDataFlow.cs:104:27:104:34 | SSA def(nonSink0) | +| GlobalDataFlow.cs:104:9:104:49 | call to method ReturnOut | out | GlobalDataFlow.cs:104:27:104:34 | SSA def(nonSink0) | +| GlobalDataFlow.cs:104:9:104:49 | call to method ReturnOut | ref | GlobalDataFlow.cs:104:27:104:34 | SSA def(nonSink0) | | GlobalDataFlow.cs:106:9:106:49 | call to method ReturnOut | out | GlobalDataFlow.cs:106:41:106:48 | SSA def(nonSink0) | | GlobalDataFlow.cs:108:9:108:49 | call to method ReturnRef | out | GlobalDataFlow.cs:108:27:108:34 | SSA def(nonSink0) | | GlobalDataFlow.cs:108:9:108:49 | call to method ReturnRef | ref | GlobalDataFlow.cs:108:27:108:34 | SSA def(nonSink0) | diff --git a/csharp/ql/test/library-tests/dataflow/global/GlobalDataFlow.cs b/csharp/ql/test/library-tests/dataflow/global/GlobalDataFlow.cs index 5f8300b1a58..41630b7fd8f 100644 --- a/csharp/ql/test/library-tests/dataflow/global/GlobalDataFlow.cs +++ b/csharp/ql/test/library-tests/dataflow/global/GlobalDataFlow.cs @@ -101,7 +101,7 @@ public class DataFlow Check(nonSink0); nonSink0 = (string)typeof(DataFlow).GetMethod("Return").Invoke(null, new object[] { nonSink0 }); Check(nonSink0); - ReturnOut("", out nonSink0, out var _); + ReturnOut("", out nonSink0, out string _); Check(nonSink0); ReturnOut(sink1, out var _, out nonSink0); Check(nonSink0); diff --git a/csharp/ql/test/library-tests/linq/Linq1.expected b/csharp/ql/test/library-tests/linq/Linq1.expected index 5415468aafd..b2405edbe0d 100644 --- a/csharp/ql/test/library-tests/linq/Linq1.expected +++ b/csharp/ql/test/library-tests/linq/Linq1.expected @@ -26,6 +26,8 @@ | queries.cs:27:11:27:42 | call to method Join | 2 | queries.cs:27:21:27:25 | access to local variable list1 | | queries.cs:27:11:27:42 | call to method Join | 3 | queries.cs:27:30:27:33 | access to local variable next | | queries.cs:27:11:27:42 | call to method Join | 4 | queries.cs:27:42:27:42 | access to local variable c | +| queries.cs:28:11:28:18 | call to method | 0 | queries.cs:27:11:27:42 | call to method Join | +| queries.cs:28:11:28:18 | call to method | 1 | queries.cs:28:18:28:18 | 1 | | queries.cs:32:11:32:21 | call to method SelectMany | 0 | queries.cs:31:11:31:25 | IList a = ... | | queries.cs:32:11:32:21 | call to method SelectMany | 1 | queries.cs:32:11:32:21 | IList b = ... | | queries.cs:32:11:32:21 | call to method SelectMany | 2 | queries.cs:32:21:32:21 | access to local variable a | @@ -49,3 +51,5 @@ | queries.cs:55:11:55:49 | call to method GroupJoin | 3 | queries.cs:55:30:55:30 | access to local variable a | | queries.cs:55:11:55:49 | call to method GroupJoin | 4 | queries.cs:55:39:55:42 | access to indexer | | queries.cs:55:11:55:49 | call to method GroupJoin | 5 | queries.cs:55:11:55:49 | IList> d = ... | +| queries.cs:56:11:56:22 | call to method | 0 | queries.cs:55:11:55:49 | call to method GroupJoin | +| queries.cs:56:11:56:22 | call to method | 1 | queries.cs:56:18:56:22 | (..., ...) | diff --git a/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected b/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected index 3989e384b6d..06473196df9 100644 --- a/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected +++ b/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected @@ -5,11 +5,13 @@ | ControlFlow.cs:9:17:9:33 | Call (unknown target) | ControlFlow.cs:9:13:9:33 | (unknown type) v | | ControlFlow.cs:10:9:10:13 | Expression | ControlFlow.cs:10:22:10:22 | access to local variable v | | ControlFlow.cs:10:9:10:43 | Call (unknown target) | ControlFlow.cs:12:9:12:87 | ...; | +| ControlFlow.cs:10:9:10:43 | call to method | ControlFlow.cs:12:9:12:87 | ...; | | ControlFlow.cs:10:9:10:44 | ...; | ControlFlow.cs:10:9:10:13 | Expression | | ControlFlow.cs:10:22:10:22 | access to local variable v | ControlFlow.cs:10:22:10:24 | Expression | | ControlFlow.cs:10:22:10:24 | Expression | ControlFlow.cs:10:22:10:26 | Expression | | ControlFlow.cs:10:22:10:26 | Expression | ControlFlow.cs:10:29:10:42 | "This is true" | | ControlFlow.cs:10:29:10:42 | "This is true" | ControlFlow.cs:10:9:10:43 | Call (unknown target) | +| ControlFlow.cs:10:29:10:42 | "This is true" | ControlFlow.cs:10:9:10:43 | call to method | | ControlFlow.cs:12:9:12:86 | Call (unknown target) | ControlFlow.cs:12:37:12:47 | Expression | | ControlFlow.cs:12:9:12:87 | ...; | ControlFlow.cs:12:9:12:86 | Call (unknown target) | | ControlFlow.cs:12:35:12:86 | { ..., ... } | ControlFlow.cs:7:10:7:10 | exit F | diff --git a/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected b/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected index edfcbf9a93b..05ae24318c3 100644 --- a/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected +++ b/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected @@ -1,6 +1,6 @@ -| errors.cs:24:31:24:40 | errors.cs:24:31:24:40 | unknown | none | +| errors.cs:24:31:24:40 | errors.cs:24:31:24:40 | call to method | none | | errors.cs:43:21:43:28 | errors.cs:43:21:43:28 | object creation of type C1 | C1 | | errors.cs:44:13:44:19 | errors.cs:44:13:44:19 | call to method m1 | m1 | | errors.cs:45:13:45:19 | errors.cs:45:13:45:19 | call to method m2 | m2 | -| errors.cs:46:13:46:38 | errors.cs:46:13:46:38 | unknown | none | +| errors.cs:46:13:46:38 | errors.cs:46:13:46:38 | call to method | none | | errors.cs:53:17:53:25 | errors.cs:53:17:53:25 | object creation of type C2 | none | diff --git a/docs/language/learn-ql/beginner/find-thief-1.rst b/docs/language/learn-ql/beginner/find-thief-1.rst index 4b4cfae9d99..3df6c108837 100644 --- a/docs/language/learn-ql/beginner/find-thief-1.rst +++ b/docs/language/learn-ql/beginner/find-thief-1.rst @@ -1,7 +1,7 @@ Find the thief: Introduction ============================ -There is a small village hidden away in the mountains. The village is divided into four parts - north, south, east, and west - and in the center stands a dark and mysterious castle... Inside the castle, locked away in the highest tower, lies the king's valuable golden crown. One night, a terrible crime is committed. A thief breaks into the tower and steals the crown! +There is a small village hidden away in the mountains. The village is divided into four parts—north, south, east, and west—and in the center stands a dark and mysterious castle... Inside the castle, locked away in the highest tower, lies the king's valuable golden crown. One night, a terrible crime is committed. A thief breaks into the tower and steals the crown! You know that the thief must live in the village, since nobody else knew about the crown. After some expert detective work, you obtain a list of all the people in the village and some of their personal details. diff --git a/docs/language/learn-ql/beginner/find-thief-2.rst b/docs/language/learn-ql/beginner/find-thief-2.rst index a29e369f12b..b31f978d36a 100644 --- a/docs/language/learn-ql/beginner/find-thief-2.rst +++ b/docs/language/learn-ql/beginner/find-thief-2.rst @@ -82,7 +82,7 @@ Notice that we have only temporarily introduced the variable ``c`` and we didn't Note - If you are familiar with logic, you may notice that ``exists`` in QL corresponds to the existential `quantifier `__ in logic. QL also has a universal quantifier ``forall(vars | formula 1 | formula 2)`` which is logically equivalent to ``not(exists(vars | formula 1 | not formula 2))``. + If you are familiar with logic, you may notice that ``exists`` in QL corresponds to the existential `quantifier `__ in logic. QL also has a universal quantifier ``forall(vars | formula 1 | formula 2)`` which is logically equivalent to ``not exists(vars | formula 1 | not formula 2)``. The real investigation ---------------------- @@ -124,8 +124,8 @@ Hints from Person t where and - not and - ... + not and + ... select t Once you have finished, you will have a list of possible suspects. One of those people must be the thief! diff --git a/docs/language/learn-ql/beginner/fire-1.rst b/docs/language/learn-ql/beginner/fire-1.rst index ee230511636..d5427b56d5f 100644 --- a/docs/language/learn-ql/beginner/fire-1.rst +++ b/docs/language/learn-ql/beginner/fire-1.rst @@ -12,50 +12,50 @@ Read the examples below to learn how to define predicates and classes in QL. The Select the southerners ---------------------- -This time you only need to consider a specific group of villagers, namely those living in the south of the village. Instead of writing ``getLocation() = "south"`` in all your queries, you could define a new `predicate `__ ``southern``: +This time you only need to consider a specific group of villagers, namely those living in the south of the village. Instead of writing ``getLocation() = "south"`` in all your queries, you could define a new `predicate `__ ``isSouthern``: .. code-block:: ql - predicate southern(Person p) { - p.getLocation() = "south" + predicate isSouthern(Person p) { + p.getLocation() = "south" } -The predicate ``southern(p)`` takes a single parameter ``p`` and checks if ``p`` satisfies the property ``p.getLocation() = "south"``. +The predicate ``isSouthern(p)`` takes a single parameter ``p`` and checks if ``p`` satisfies the property ``p.getLocation() = "south"``. .. pull-quote:: Note - The name of a predicate always starts with a lowercase letter. - - You can also define predicates with a result. In that case, the keyword ``predicate`` is replaced with the type of the result. This is like introducing a new argument, the special variable ``result``. For example, ``int getAge() {result = ...}`` returns an ``int``. + - You can also define predicates with a result. In that case, the keyword ``predicate`` is replaced with the type of the result. This is like introducing a new argument, the special variable ``result``. For example, ``int getAge() { result = ... }`` returns an ``int``. You can now list all southerners using: .. code-block:: ql - /* define predicate `southern` as above */ + /* define predicate `isSouthern` as above */ from Person p - where southern(p) + where isSouthern(p) select p -This is already a nice way to simplify the logic, but we could be more efficient. Currently, the query looks at every ``Person p``, and then restricts to those who satisfy ``southern(p)``. Instead, we could define a new `class `__ ``Southerner`` containing precisely the people we want to consider. +This is already a nice way to simplify the logic, but we could be more efficient. Currently, the query looks at every ``Person p``, and then restricts to those who satisfy ``isSouthern(p)``. Instead, we could define a new `class `__ ``Southerner`` containing precisely the people we want to consider. .. code-block:: ql class Southerner extends Person { - Southerner() { southern(this) } + Southerner() { isSouthern(this) } } -A class in QL represents a logical property: when a value satisfies that property, it is a member of the class. This means that a value can be in many classes - being in a particular class doesn't stop it from being in other classes too. +A class in QL represents a logical property: when a value satisfies that property, it is a member of the class. This means that a value can be in many classes—being in a particular class doesn't stop it from being in other classes too. -The expression ``southern(this)`` defines the logical property represented by the class, called its *characteristic predicate*. It uses a special variable ``this`` and indicates that a ``Person`` "``this``" is a ``Southerner`` if the property ``southern(this)`` holds. +The expression ``isSouthern(this)`` defines the logical property represented by the class, called its *characteristic predicate*. It uses a special variable ``this`` and indicates that a ``Person`` "``this``" is a ``Southerner`` if the property ``isSouthern(this)`` holds. .. pull-quote:: Note - If you are familiar with object-oriented programming languages, you might be tempted to think of the characteristic predicate as a *constructor*. However, this is **not** the case - it is a logical property which does not create any objects. + If you are familiar with object-oriented programming languages, you might be tempted to think of the characteristic predicate as a *constructor*. However, this is **not** the case—it is a logical property which does not create any objects. You always need to define a class in QL in terms of an existing (larger) class. In our example, a ``Southerner`` is a special kind of ``Person``, so we say that ``Southerner`` *extends* ("is a subset of") ``Person``. @@ -66,7 +66,7 @@ Using this class you can now list all people living in the south simply as: from Southerner s select s -You may have noticed that some predicates are appended, for example ``p.getAge()``, while others are not, for example ``southern(p)``. This is because ``getAge()`` is a member predicate, that is, a predicate that only applies to members of a class. You define such a member predicate inside a class. In this case, ``getAge()`` is defined inside the class ``Person``. In contrast, ``southern`` is defined separately and is not inside any classes. Member predicates are especially useful because you can chain them together easily. For example, ``p.getAge().sqrt()`` first gets the age of ``p`` and then calculates the square root of that number. +You may have noticed that some predicates are appended, for example ``p.getAge()``, while others are not, for example ``isSouthern(p)``. This is because ``getAge()`` is a member predicate, that is, a predicate that only applies to members of a class. You define such a member predicate inside a class. In this case, ``getAge()`` is defined inside the class ``Person``. In contrast, ``isSouthern`` is defined separately and is not inside any classes. Member predicates are especially useful because you can chain them together easily. For example, ``p.getAge().sqrt()`` first gets the age of ``p`` and then calculates the square root of that number. Travel restrictions ------------------- @@ -88,20 +88,19 @@ Start by defining a class ``Child`` containing all villagers under 10 years old. .. code-block:: ql class Child extends Person { + /* the characteristic predicate */ + Child() { this.getAge() < 10 } - /* the characteristic predicate */ - Child() { this.getAge() < 10 } - - /* a member predicate */ - override predicate isAllowedIn(string region) { - region = this.getLocation() - } + /* a member predicate */ + override predicate isAllowedIn(string region) { + region = this.getLocation() + } } Now try applying ``isAllowedIn(string region)`` to a person ``p``. If ``p`` is not a child, the original definition is used, but if ``p`` is a child, the new predicate definition overrides the original. You know that the fire starters live in the south *and* that they must have been able to travel to the north. Write a query to find the possible suspects. You could also extend the ``select`` clause to list the age of the suspects. That way you can clearly see that all the children have been excluded from the list. -➤ `See the answer in the query console `__ +➤ `See the answer in the query console `__ Continue to the :doc:`next page ` to gather more clues and find out which of your suspects started the fire... diff --git a/docs/language/learn-ql/beginner/fire-2.rst b/docs/language/learn-ql/beginner/fire-2.rst index 0c7e4852ae7..0f5312e69f0 100644 --- a/docs/language/learn-ql/beginner/fire-2.rst +++ b/docs/language/learn-ql/beginner/fire-2.rst @@ -11,27 +11,27 @@ This is a very helpful clue. Remember that you wrote a QL query to select all ba where not exists (string c | p.getHairColor() = c) select p -To avoid having to type ``not exists (string c | p.getHairColor() = c)`` every time you want to select a bald person, you can instead define another new predicate ``bald``. +To avoid having to type ``not exists (string c | p.getHairColor() = c)`` every time you want to select a bald person, you can instead define another new predicate ``isBald``. .. code-block:: ql - predicate bald(Person p) { - not exists (string c | p.getHairColor() = c) + predicate isBald(Person p) { + not exists (string c | p.getHairColor() = c) } -The property ``bald(p)`` holds whenever ``p`` is bald, so you can replace the previous query with: +The property ``isBald(p)`` holds whenever ``p`` is bald, so you can replace the previous query with: .. code-block:: ql from Person p - where bald(p) + where isBald(p) select p -The predicate ``bald`` is defined to take a ``Person``, so it can also take a ``Southerner``, as ``Southerner`` is a subtype of ``Person``. It can't take an ``int`` for example - that would cause an error. +The predicate ``isBald`` is defined to take a ``Person``, so it can also take a ``Southerner``, as ``Southerner`` is a subtype of ``Person``. It can't take an ``int`` for example—that would cause an error. You can now write a query to select the bald southerners who are allowed into the north. -➤ `See the answer in the query console `__ +➤ `See the answer in the query console `__ You have found the two fire starters! They are arrested and the villagers are once again impressed with your work. diff --git a/docs/language/learn-ql/beginner/heir.rst b/docs/language/learn-ql/beginner/heir.rst index f0f0c315025..42589f03125 100644 --- a/docs/language/learn-ql/beginner/heir.rst +++ b/docs/language/learn-ql/beginner/heir.rst @@ -1,9 +1,9 @@ Crown the rightful heir ======================= -Phew! No more crimes in the village - you can finally leave the village and go home. +Phew! No more crimes in the village—you can finally leave the village and go home. -But then... During your last night in the village, the old king - the great King Basil - dies in his sleep and there is chaos everywhere! +But then... During your last night in the village, the old king—the great King Basil—dies in his sleep and there is chaos everywhere! The king never married and he had no children, so nobody knows who should inherit the king's castle and fortune. Immediately, lots of villagers claim that they are somehow descended from the king's family and that they are the true heir. People argue and fight and the situation seems hopeless. @@ -38,8 +38,8 @@ We know that the king has no children himself, but perhaps he has siblings. Writ .. code-block:: ql from Person p - where parentOf(p) = parentOf("King Basil") and - not p = "King Basil" + where parentOf(p) = parentOf("King Basil") and + not p = "King Basil" select p He does indeed have siblings! But you need to check if any of them are alive... Here is one more predicate you might need: @@ -56,8 +56,8 @@ Use this predicate to see if the any of the king's siblings are alive. from Person p where parentOf(p) = parentOf("King Basil") and - not p = "King Basil" - and not p.isDeceased() + not p = "King Basil" + and not p.isDeceased() select p Unfortunately, none of King Basil's siblings are alive. Time to investigate further. It might be helpful to define a predicate ``childOf()`` which returns a child of the person. To do this, the ``parentOf()`` predicate can be used inside the definition of ``childOf()``. Remember that someone is a child of ``p`` if and only if ``p`` is their parent: @@ -65,7 +65,7 @@ Unfortunately, none of King Basil's siblings are alive. Time to investigate furt .. code-block:: ql Person childOf(Person p) { - p = parentOf(result) + p = parentOf(result) } .. pull-quote:: @@ -80,7 +80,7 @@ Try to write a query to find out if any of the king's siblings have children: from Person p where parentOf(p) = parentOf("King Basil") and - not p = "King Basil" + not p = "King Basil" select childOf(p) The query returns no results, so they have no children. But perhaps King Basil has a cousin who is alive or has children, or a second cousin, or... @@ -100,8 +100,8 @@ You can translate this into QL as follows: .. code-block:: ql Person ancestorOf(Person p) { - result = parentOf(p) or - result = parentOf(ancestorOf(p)) + result = parentOf(p) or + result = parentOf(ancestorOf(p)) } As you can see, you have used the predicate ``ancestorOf()`` inside its own definition. This is an example of `recursion `__. @@ -120,12 +120,12 @@ Here is one way to define ``relativeOf()``: .. code-block:: ql Person relativeOf(Person p) { - parentOf*(result) = parentOf*(p) + parentOf*(result) = parentOf*(p) } Don't forget to use the predicate ``isDeceased()`` to find relatives that are still alive. -➤ `See the answer in the query console `__ +➤ `See the answer in the query console `__ Select the true heir -------------------- @@ -136,9 +136,9 @@ To decide who should inherit the king's fortune, the villagers carefully read th *"The heir to the throne is the closest living relative of the king. Any person with a criminal record will not be considered. If there are multiple candidates, the oldest person is the heir."* -As your final challenge, define a predicate ``criminalRecord`` so that ``criminalRecord(p)`` holds if ``p`` is any of the criminals you unmasked earlier (in the :doc:`Find the thief ` and :doc:`Catch the fire starter ` tutorials). +As your final challenge, define a predicate ``hasCriminalRecord`` so that ``hasCriminalRecord(p)`` holds if ``p`` is any of the criminals you unmasked earlier (in the :doc:`Find the thief ` and :doc:`Catch the fire starter ` tutorials). -➤ `See the answer in the query console `__ +➤ `See the answer in the query console `__ Experimental explorations ------------------------- diff --git a/docs/language/learn-ql/beginner/ql-tutorials.rst b/docs/language/learn-ql/beginner/ql-tutorials.rst index d97c4d6688e..2f3d6cc5e5d 100644 --- a/docs/language/learn-ql/beginner/ql-tutorials.rst +++ b/docs/language/learn-ql/beginner/ql-tutorials.rst @@ -16,9 +16,9 @@ some simple examples. Currently the following detective tutorials are available: -- :doc:`Find the thief ` - a three part mystery that introduces logical connectives, quantifiers, and aggregates -- :doc:`Catch the fire starter ` - an intriguing search that introduces predicates and classes -- :doc:`Crown the rightful heir ` - a detective puzzle that introduces recursion +- :doc:`Find the thief `—a three part mystery that introduces logical connectives, quantifiers, and aggregates +- :doc:`Catch the fire starter `—an intriguing search that introduces predicates and classes +- :doc:`Crown the rightful heir `—a detective puzzle that introduces recursion Further resources ----------------- diff --git a/java/ql/src/Violations of Best Practice/Boxed Types/BoxedVariable.ql b/java/ql/src/Violations of Best Practice/Boxed Types/BoxedVariable.ql index 4f8ed49e010..a5523845c24 100644 --- a/java/ql/src/Violations of Best Practice/Boxed Types/BoxedVariable.ql +++ b/java/ql/src/Violations of Best Practice/Boxed Types/BoxedVariable.ql @@ -39,6 +39,9 @@ predicate notDeliberatelyBoxed(LocalBoxedVar v) { ) } +pragma[nomagic] +int callableGetNumberOfParameters(Callable c) { result = c.getNumberOfParameters() } + /** * Replacing the type of a boxed variable with the corresponding primitive type may affect * overload resolution. If this is the case then the boxing is most likely intentional and @@ -52,7 +55,7 @@ predicate affectsOverload(LocalBoxedVar v) { c1.getParameterType(i) instanceof RefType and c2.getParameterType(i) instanceof PrimitiveType and c1.getName() = c2.getName() and - c1.getNumberOfParameters() = c2.getNumberOfParameters() + callableGetNumberOfParameters(c1) = callableGetNumberOfParameters(c2) ) } diff --git a/java/ql/src/semmle/code/Location.qll b/java/ql/src/semmle/code/Location.qll index 92b92fc69ed..a34bba9ddc7 100755 --- a/java/ql/src/semmle/code/Location.qll +++ b/java/ql/src/semmle/code/Location.qll @@ -84,6 +84,7 @@ class Top extends @top { int getNumberOfCommentLines() { numlines(this, _, _, result) } /** Gets a textual representation of this element. */ + cached string toString() { hasName(this, result) } } diff --git a/java/ql/src/semmle/code/java/deadcode/DeadCode.qll b/java/ql/src/semmle/code/java/deadcode/DeadCode.qll index 9c7d7ba4a53..9749413a23c 100644 --- a/java/ql/src/semmle/code/java/deadcode/DeadCode.qll +++ b/java/ql/src/semmle/code/java/deadcode/DeadCode.qll @@ -301,7 +301,7 @@ class RootdefCallable extends Callable { } } -pragma[noinline] +pragma[nomagic] private predicate overrideAccess(Callable c, int i) { exists(Method m | m.overridesOrInstantiates+(c) | exists(m.getParameter(i).getAnAccess())) } diff --git a/java/ql/src/semmle/code/java/frameworks/gwt/GWT.qll b/java/ql/src/semmle/code/java/frameworks/gwt/GWT.qll index ed68cd395fb..c704320657f 100644 --- a/java/ql/src/semmle/code/java/frameworks/gwt/GWT.qll +++ b/java/ql/src/semmle/code/java/frameworks/gwt/GWT.qll @@ -61,19 +61,29 @@ class ClientSideGwtCompilationUnit extends GwtCompilationUnit { } } -/** Auxiliary predicate: `jsni` is a JSNI comment associated with method `m`. */ -private predicate jsniComment(Javadoc jsni, Method m) { +private predicate jsni(Javadoc jsni, File file, int startline) { // The comment must start with `-{` ... jsni.getChild(0).getText().matches("-{%") and // ... and it must end with `}-`. jsni.getChild(jsni.getNumChild() - 1).getText().matches("%}-") and - // The associated callable must be marked as `native` ... + file = jsni.getFile() and + startline = jsni.getLocation().getStartLine() +} + +private predicate nativeMethodLines(Method m, File file, int line) { m.isNative() and - // ... and the comment has to be contained in `m`. - jsni.getFile() = m.getFile() and - jsni.getLocation().getStartLine() in [m.getLocation().getStartLine() .. m - .getLocation() - .getEndLine()] + file = m.getFile() and + line in [m.getLocation().getStartLine() .. m.getLocation().getEndLine()] +} + +/** Auxiliary predicate: `jsni` is a JSNI comment associated with method `m`. */ +private predicate jsniComment(Javadoc jsni, Method m) { + exists(File file, int line | + jsni(jsni, file, line) and + // The associated callable must be marked as `native` + // and the comment has to be contained in `m`. + nativeMethodLines(m, file, line) + ) } /** diff --git a/java/ql/src/semmle/code/xml/XML.qll b/java/ql/src/semmle/code/xml/XML.qll index b4fe2c83417..c6d55c8190b 100755 --- a/java/ql/src/semmle/code/xml/XML.qll +++ b/java/ql/src/semmle/code/xml/XML.qll @@ -2,7 +2,7 @@ * Provides classes and predicates for working with XML files and their content. */ -import semmle.code.Location +import semmle.files.FileSystem /** An XML element that has a location. */ abstract class XMLLocatable extends @xmllocatable { @@ -34,6 +34,12 @@ abstract class XMLLocatable extends @xmllocatable { * both of which can contain other elements. */ class XMLParent extends @xmlparent { + XMLParent() { + // explicitly restrict `this` to be either an `XMLElement` or an `XMLFile`; + // the type `@xmlparent` currently also includes non-XML files + this instanceof @xmlelement or xmlEncoding(this, _) + } + /** * Gets a printable representation of this XML parent. * (Intended to be overridden in subclasses.) @@ -82,7 +88,10 @@ class XMLParent extends @xmlparent { ) } - /** Append all the character sequences of this XML parent from left to right, separated by a space. */ + /** + * Gets the result of appending all the character sequences of this XML parent from + * left to right, separated by a space. + */ string allCharactersString() { result = concat(string chars, int pos | xmlChars(_, chars, this, pos, _, _) @@ -108,6 +117,20 @@ class XMLFile extends XMLParent, File { /** Gets the name of this XML file. */ override string getName() { result = File.super.getAbsolutePath() } + /** + * DEPRECATED: Use `getAbsolutePath()` instead. + * + * Gets the path of this XML file. + */ + deprecated string getPath() { result = getAbsolutePath() } + + /** + * DEPRECATED: Use `getParentContainer().getAbsolutePath()` instead. + * + * Gets the path of the folder that contains this XML file. + */ + deprecated string getFolder() { result = getParentContainer().getAbsolutePath() } + /** Gets the encoding of this XML file. */ string getEncoding() { xmlEncoding(this, result) } @@ -121,7 +144,17 @@ class XMLFile extends XMLParent, File { XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) } } -/** A "Document Type Definition" of an XML file. */ +/** + * An XML document type definition (DTD). + * + * Example: + * + * ``` + * + * + * + * ``` + */ class XMLDTD extends @xmldtd { /** Gets the name of the root element of this DTD. */ string getRoot() { xmlDTDs(this, result, _, _, _) } @@ -148,7 +181,17 @@ class XMLDTD extends @xmldtd { } } -/** An XML tag in an XML file. */ +/** + * An XML element in an XML file. + * + * Example: + * + * ``` + * + * + * ``` + */ class XMLElement extends @xmlelement, XMLParent, XMLLocatable { /** Holds if this XML element has the given `name`. */ predicate hasName(string name) { name = getName() } @@ -193,7 +236,16 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable { override string toString() { result = XMLParent.super.toString() } } -/** An attribute that occurs inside an XML element. */ +/** + * An attribute that occurs inside an XML element. + * + * Examples: + * + * ``` + * package="com.example.exampleapp" + * android:versionCode="1" + * ``` + */ class XMLAttribute extends @xmlattribute, XMLLocatable { /** Gets the name of this attribute. */ string getName() { xmlAttrs(this, _, result, _, _, _) } @@ -214,7 +266,15 @@ class XMLAttribute extends @xmlattribute, XMLLocatable { override string toString() { result = this.getName() + "=" + this.getValue() } } -/** A namespace used in an XML file */ +/** + * A namespace used in an XML file. + * + * Example: + * + * ``` + * xmlns:android="http://schemas.android.com/apk/res/android" + * ``` + */ class XMLNamespace extends @xmlnamespace { /** Gets the prefix of this namespace. */ string getPrefix() { xmlNs(this, result, _, _) } @@ -233,7 +293,15 @@ class XMLNamespace extends @xmlnamespace { } } -/** A comment of the form `` is an XML comment. */ +/** + * A comment in an XML file. + * + * Example: + * + * ``` + * + * ``` + */ class XMLComment extends @xmlcomment, XMLLocatable { /** Gets the text content of this XML comment. */ string getText() { xmlComments(this, result, _, _) } @@ -248,6 +316,12 @@ class XMLComment extends @xmlcomment, XMLLocatable { /** * A sequence of characters that occurs between opening and * closing tags of an XML element, excluding other elements. + * + * Example: + * + * ``` + * This is a sequence of characters. + * ``` */ class XMLCharacters extends @xmlcharacters, XMLLocatable { /** Gets the content of this character sequence. */ diff --git a/javascript/extractor/src/com/semmle/jcorn/ESNextParser.java b/javascript/extractor/src/com/semmle/jcorn/ESNextParser.java index 93295a519db..f750013067b 100644 --- a/javascript/extractor/src/com/semmle/jcorn/ESNextParser.java +++ b/javascript/extractor/src/com/semmle/jcorn/ESNextParser.java @@ -240,6 +240,9 @@ public class ESNextParser extends JSXParser { if (this.type == TokenType._import) { Position startLoc = this.startLoc; this.next(); + if (this.eat(TokenType.dot)) { + return parseImportMeta(startLoc); + } this.expect(TokenType.parenL); return parseDynamicImport(startLoc); } @@ -413,6 +416,19 @@ public class ESNextParser extends JSXParser { } } + /** + * Parses an import.meta expression, assuming that the initial "import" and "." has been consumed. + */ + private MetaProperty parseImportMeta(Position loc) { + Position propertyLoc = this.startLoc; + Identifier property = this.parseIdent(true); + if (!property.getName().equals("meta")) { + this.unexpected(propertyLoc); + } + return this.finishNode( + new MetaProperty(new SourceLocation(loc), new Identifier(new SourceLocation(loc), "import"), property)); + } + /** * Parses a dynamic import, assuming that the keyword `import` and the opening parenthesis have * already been consumed. diff --git a/javascript/extractor/src/com/semmle/js/ast/MetaProperty.java b/javascript/extractor/src/com/semmle/js/ast/MetaProperty.java index 1a1154a53a5..cc98b94b406 100644 --- a/javascript/extractor/src/com/semmle/js/ast/MetaProperty.java +++ b/javascript/extractor/src/com/semmle/js/ast/MetaProperty.java @@ -3,8 +3,8 @@ package com.semmle.js.ast; /** * A meta property access (cf. ECMAScript 2015 Language Specification, Chapter 12.3.8). * - *

Currently the only recognised meta properties are new.target and - * function.sent. + *

Currently the only recognised meta properties are new.target, + * import.meta and function.sent. */ public class MetaProperty extends Expression { private final Identifier meta, property; diff --git a/javascript/ql/src/semmle/javascript/XML.qll b/javascript/ql/src/semmle/javascript/XML.qll index af06cedcbcd..c6d55c8190b 100755 --- a/javascript/ql/src/semmle/javascript/XML.qll +++ b/javascript/ql/src/semmle/javascript/XML.qll @@ -2,7 +2,7 @@ * Provides classes and predicates for working with XML files and their content. */ -import javascript +import semmle.files.FileSystem /** An XML element that has a location. */ abstract class XMLLocatable extends @xmllocatable { @@ -88,7 +88,10 @@ class XMLParent extends @xmlparent { ) } - /** Append all the character sequences of this XML parent from left to right, separated by a space. */ + /** + * Gets the result of appending all the character sequences of this XML parent from + * left to right, separated by a space. + */ string allCharactersString() { result = concat(string chars, int pos | xmlChars(_, chars, this, pos, _, _) @@ -114,6 +117,20 @@ class XMLFile extends XMLParent, File { /** Gets the name of this XML file. */ override string getName() { result = File.super.getAbsolutePath() } + /** + * DEPRECATED: Use `getAbsolutePath()` instead. + * + * Gets the path of this XML file. + */ + deprecated string getPath() { result = getAbsolutePath() } + + /** + * DEPRECATED: Use `getParentContainer().getAbsolutePath()` instead. + * + * Gets the path of the folder that contains this XML file. + */ + deprecated string getFolder() { result = getParentContainer().getAbsolutePath() } + /** Gets the encoding of this XML file. */ string getEncoding() { xmlEncoding(this, result) } diff --git a/javascript/ql/test/library-tests/Modules/ImportMeta.qll b/javascript/ql/test/library-tests/Modules/ImportMeta.qll new file mode 100644 index 00000000000..81a4ba1a086 --- /dev/null +++ b/javascript/ql/test/library-tests/Modules/ImportMeta.qll @@ -0,0 +1,5 @@ +import semmle.javascript.ES2015Modules + +query predicate test_ImportMetaExpr(ImportMetaExpr meta) { + any() +} diff --git a/javascript/ql/test/library-tests/Modules/importmeta.js b/javascript/ql/test/library-tests/Modules/importmeta.js new file mode 100644 index 00000000000..05bc2f96e49 --- /dev/null +++ b/javascript/ql/test/library-tests/Modules/importmeta.js @@ -0,0 +1 @@ +var foo = new URL('../cli.svg', import.meta.url).pathname; \ No newline at end of file diff --git a/javascript/ql/test/library-tests/Modules/tests.expected b/javascript/ql/test/library-tests/Modules/tests.expected index 9de217ec11b..120038a8a24 100644 --- a/javascript/ql/test/library-tests/Modules/tests.expected +++ b/javascript/ql/test/library-tests/Modules/tests.expected @@ -26,6 +26,8 @@ test_getExportedName | m/c.js:5:10:5:15 | g as h | g | test_OtherImports | es2015_require.js:1:11:1:24 | require('./d') | d.js:1:1:5:0 | | +test_ImportMetaExpr +| importmeta.js:1:33:1:43 | import.meta | test_ReExportDeclarations | b.js:7:1:7:21 | export ... './a'; | b.js:7:16:7:20 | './a' | | d.js:4:1:4:20 | export * from 'm/c'; | d.js:4:15:4:19 | 'm/c' | @@ -102,6 +104,7 @@ test_NamedImportSpecifier test_GlobalVariableRef | a.js:5:31:5:31 | o | | exports.js:3:9:3:15 | exports | +| importmeta.js:1:15:1:17 | URL | | tst.html:6:3:6:7 | alert | test_BulkReExportDeclarations | d.js:4:1:4:20 | export * from 'm/c'; | diff --git a/javascript/ql/test/library-tests/Modules/tests.ql b/javascript/ql/test/library-tests/Modules/tests.ql index 7f1652f60ae..447cfd97496 100644 --- a/javascript/ql/test/library-tests/Modules/tests.ql +++ b/javascript/ql/test/library-tests/Modules/tests.ql @@ -2,6 +2,7 @@ import ImportSpecifiers import getLocalName import getExportedName import OtherImports +import ImportMeta import ReExportDeclarations import ImportDefaultSpecifiers import getImportedName diff --git a/python/ql/src/semmle/python/objects/ObjectAPI.qll b/python/ql/src/semmle/python/objects/ObjectAPI.qll index c6eb6db6517..ac5348264cf 100644 --- a/python/ql/src/semmle/python/objects/ObjectAPI.qll +++ b/python/ql/src/semmle/python/objects/ObjectAPI.qll @@ -211,7 +211,7 @@ module Value { } /** Gets the `Value` for the integer constant `i`, if it exists. - * There will be no `Value` for most integers, but the following are + * There will be no `Value` for most integers, but the following are * guaranteed to exist: * * From zero to 511 inclusive. * * All powers of 2 (up to 2**30) @@ -486,6 +486,11 @@ class PythonFunctionValue extends FunctionValue { ) } + /** Gets a control flow node corresponding to a return statement in this function */ + ControlFlowNode getAReturnedNode() { + result = this.getScope().getAReturnValueFlowNode() + } + } /** Class representing builtin functions, such as `len` or `print` */ diff --git a/python/ql/src/semmle/python/web/flask/Response.qll b/python/ql/src/semmle/python/web/flask/Response.qll index 2ae78d2cdfc..0828c180e9a 100644 --- a/python/ql/src/semmle/python/web/flask/Response.qll +++ b/python/ql/src/semmle/python/web/flask/Response.qll @@ -9,8 +9,8 @@ import semmle.python.web.flask.General */ class FlaskRoutedResponse extends HttpResponseTaintSink { FlaskRoutedResponse() { - exists(PyFunctionObject response | - flask_routing(_, response.getFunction()) and + exists(PythonFunctionValue response | + flask_routing(_, response.getScope()) and this = response.getAReturnedNode() ) } diff --git a/python/ql/src/semmle/python/web/pyramid/Response.qll b/python/ql/src/semmle/python/web/pyramid/Response.qll index 511d1e28322..37dc4be783c 100644 --- a/python/ql/src/semmle/python/web/pyramid/Response.qll +++ b/python/ql/src/semmle/python/web/pyramid/Response.qll @@ -11,8 +11,8 @@ private import semmle.python.web.Http */ class PyramidRoutedResponse extends HttpResponseTaintSink { PyramidRoutedResponse() { - exists(PyFunctionObject view | - is_pyramid_view_function(view.getFunction()) and + exists(PythonFunctionValue view | + is_pyramid_view_function(view.getScope()) and this = view.getAReturnedNode() ) } diff --git a/python/ql/src/semmle/python/web/twisted/Request.qll b/python/ql/src/semmle/python/web/twisted/Request.qll index 287a33ab1fe..67bb8cedbfe 100644 --- a/python/ql/src/semmle/python/web/twisted/Request.qll +++ b/python/ql/src/semmle/python/web/twisted/Request.qll @@ -1,53 +1,35 @@ import python - import semmle.python.security.TaintTracking import semmle.python.web.Http import Twisted /** A twisted.web.http.Request object */ class TwistedRequest extends TaintKind { - - TwistedRequest() { - this = "twisted.request.http.Request" - } + TwistedRequest() { this = "twisted.request.http.Request" } override TaintKind getTaintOfAttribute(string name) { result instanceof ExternalStringSequenceDictKind and - ( - name = "args" - ) + name = "args" or result instanceof ExternalStringKind and - ( - name = "uri" - ) + name = "uri" } override TaintKind getTaintOfMethodResult(string name) { - ( - name = "getHeader" or - name = "getCookie" or - name = "getUser" or - name = "getPassword" - ) and - result instanceof ExternalStringKind + ( + name = "getHeader" or + name = "getCookie" or + name = "getUser" or + name = "getPassword" + ) and + result instanceof ExternalStringKind } - } - class TwistedRequestSource extends TaintSource { + TwistedRequestSource() { isTwistedRequestInstance(this) } - TwistedRequestSource() { - isTwistedRequestInstance(this) - } - - override string toString() { - result = "Twisted request source" - } - - override predicate isSourceOf(TaintKind kind) { - kind instanceof TwistedRequest - } + override string toString() { result = "Twisted request source" } + override predicate isSourceOf(TaintKind kind) { kind instanceof TwistedRequest } } diff --git a/python/ql/src/semmle/python/web/twisted/Response.qll b/python/ql/src/semmle/python/web/twisted/Response.qll index cdf35933e61..435960c40ab 100644 --- a/python/ql/src/semmle/python/web/twisted/Response.qll +++ b/python/ql/src/semmle/python/web/twisted/Response.qll @@ -1,5 +1,4 @@ import python - import semmle.python.security.TaintTracking import semmle.python.web.Http import semmle.python.security.strings.Basic @@ -8,7 +7,7 @@ import Request class TwistedResponse extends TaintSink { TwistedResponse() { - exists(PyFunctionObject func, string name | + exists(PythonFunctionValue func, string name, Return ret | isKnownRequestHandlerMethodName(name) and name = func.getName() and func = getTwistedRequestHandlerMethod(name) and @@ -16,13 +15,9 @@ class TwistedResponse extends TaintSink { ) } - override predicate sinks(TaintKind kind) { - kind instanceof ExternalStringKind - } + override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } - override string toString() { - result = "Twisted response" - } + override string toString() { result = "Twisted response" } } /** @@ -30,7 +25,7 @@ class TwistedResponse extends TaintSink { * object, which affects the properties of the subsequent response sent to this * request. */ - class TwistedRequestSetter extends HttpResponseTaintSink { +class TwistedRequestSetter extends HttpResponseTaintSink { TwistedRequestSetter() { exists(CallNode call, ControlFlowNode node, string name | ( @@ -44,11 +39,7 @@ class TwistedResponse extends TaintSink { ) } - override predicate sinks(TaintKind kind) { - kind instanceof ExternalStringKind - } + override predicate sinks(TaintKind kind) { kind instanceof ExternalStringKind } - override string toString() { - result = "Twisted request setter" - } -} \ No newline at end of file + override string toString() { result = "Twisted request setter" } +} diff --git a/python/ql/src/semmle/python/web/twisted/Twisted.qll b/python/ql/src/semmle/python/web/twisted/Twisted.qll index 25912ad523e..e3b0ab0f9be 100644 --- a/python/ql/src/semmle/python/web/twisted/Twisted.qll +++ b/python/ql/src/semmle/python/web/twisted/Twisted.qll @@ -1,20 +1,17 @@ import python - import semmle.python.security.TaintTracking -private ClassObject theTwistedHttpRequestClass() { - result = ModuleObject::named("twisted.web.http").attr("Request") +private ClassValue theTwistedHttpRequestClass() { + result = Value::named("twisted.web.http.Request") } -private ClassObject theTwistedHttpResourceClass() { - result = ModuleObject::named("twisted.web.resource").attr("Resource") +private ClassValue theTwistedHttpResourceClass() { + result = Value::named("twisted.web.resource.Resource") } -ClassObject aTwistedRequestHandlerClass() { - result.getASuperType() = theTwistedHttpResourceClass() -} +ClassValue aTwistedRequestHandlerClass() { result.getABaseType+() = theTwistedHttpResourceClass() } -FunctionObject getTwistedRequestHandlerMethod(string name) { +FunctionValue getTwistedRequestHandlerMethod(string name) { result = aTwistedRequestHandlerClass().declaredAttribute(name) } @@ -24,29 +21,30 @@ predicate isKnownRequestHandlerMethodName(string name) { name.matches("render_%") } -/** Holds if `node` is likely to refer to an instance of the twisted +/** + * Holds if `node` is likely to refer to an instance of the twisted * `Request` class. */ predicate isTwistedRequestInstance(NameNode node) { - node.refersTo(_, theTwistedHttpRequestClass(), _) + node.pointsTo().getClass() = theTwistedHttpRequestClass() or - /* In points-to analysis cannot infer that a given object is an instance of + /* + * In points-to analysis cannot infer that a given object is an instance of * the `twisted.web.http.Request` class, we also include any parameter * called `request` that appears inside a subclass of a request handler * class, and the appropriate arguments of known request handler methods. */ - exists(Function func | func = node.getScope() | - func.getEnclosingScope().(Class).getClassObject() = aTwistedRequestHandlerClass() - ) and - ( - /* Any parameter called `request` */ - node.getId() = "request" and - node.isParameter() - or - /* Any request parameter of a known request handler method */ - exists(FunctionObject func | node.getScope() = func.getFunction() | + + exists(Function func | + func = node.getScope() and + func.getEnclosingScope() = aTwistedRequestHandlerClass().getScope() + | + /* Any parameter called `request` */ + node.getId() = "request" and + node.isParameter() + or + /* Any request parameter of a known request handler method */ isKnownRequestHandlerMethodName(func.getName()) and - node.getNode() = func.getFunction().getArg(1) - ) + node.getNode() = func.getArg(1) ) } diff --git a/python/ql/src/semmle/python/xml/XML.qll b/python/ql/src/semmle/python/xml/XML.qll index 13cc8d84f37..c6d55c8190b 100755 --- a/python/ql/src/semmle/python/xml/XML.qll +++ b/python/ql/src/semmle/python/xml/XML.qll @@ -1,26 +1,31 @@ /** - * A library for working with XML files and their content. + * Provides classes and predicates for working with XML files and their content. */ -import semmle.python.Files +import semmle.files.FileSystem /** An XML element that has a location. */ abstract class XMLLocatable extends @xmllocatable { - /** The source location for this element. */ - Location getLocation() { xmllocations(this,result) } + /** Gets the source location for this element. */ + Location getLocation() { xmllocations(this, result) } /** - * Whether this element has the specified location information, - * including file path, start line, start column, end line and end column. + * Holds if this element is at the specified location. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. + * For more information, see + * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html). */ - predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) { + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { exists(File f, Location l | l = this.getLocation() | - locations_default(l,f,startline,startcolumn,endline,endcolumn) and - filepath = f.getName() + locations_default(l, f, startline, startcolumn, endline, endcolumn) and + filepath = f.getAbsolutePath() ) } - /** A printable representation of this element. */ + /** Gets a textual representation of this element. */ abstract string toString(); } @@ -29,254 +34,305 @@ abstract class XMLLocatable extends @xmllocatable { * both of which can contain other elements. */ class XMLParent extends @xmlparent { + XMLParent() { + // explicitly restrict `this` to be either an `XMLElement` or an `XMLFile`; + // the type `@xmlparent` currently also includes non-XML files + this instanceof @xmlelement or xmlEncoding(this, _) + } + /** - * A printable representation of this XML parent. + * Gets a printable representation of this XML parent. * (Intended to be overridden in subclasses.) */ - /*abstract*/ string getName() { result = "parent" } + abstract string getName(); - /** The file to which this XML parent belongs. */ - XMLFile getFile() { result = this or xmlElements(this,_,_,_,result) } + /** Gets the file to which this XML parent belongs. */ + XMLFile getFile() { result = this or xmlElements(this, _, _, _, result) } - /** The child element at a specified index of this XML parent. */ + /** Gets the child element at a specified index of this XML parent. */ XMLElement getChild(int index) { xmlElements(result, _, this, index, _) } - /** A child element of this XML parent. */ - XMLElement getAChild() { xmlElements(result,_,this,_,_) } + /** Gets a child element of this XML parent. */ + XMLElement getAChild() { xmlElements(result, _, this, _, _) } - /** A child element of this XML parent with the given `name`. */ - XMLElement getAChild(string name) { xmlElements(result,_,this,_,_) and result.hasName(name) } + /** Gets a child element of this XML parent with the given `name`. */ + XMLElement getAChild(string name) { xmlElements(result, _, this, _, _) and result.hasName(name) } - /** A comment that is a child of this XML parent. */ - XMLComment getAComment() { xmlComments(result,_,this,_) } + /** Gets a comment that is a child of this XML parent. */ + XMLComment getAComment() { xmlComments(result, _, this, _) } - /** A character sequence that is a child of this XML parent. */ - XMLCharacters getACharactersSet() { xmlChars(result,_,this,_,_,_) } + /** Gets a character sequence that is a child of this XML parent. */ + XMLCharacters getACharactersSet() { xmlChars(result, _, this, _, _, _) } - /** The depth in the tree. (Overridden in XMLElement.) */ + /** Gets the depth in the tree. (Overridden in XMLElement.) */ int getDepth() { result = 0 } - /** The number of child XML elements of this XML parent. */ - int getNumberOfChildren() { - result = count(XMLElement e | xmlElements(e,_,this,_,_)) - } + /** Gets the number of child XML elements of this XML parent. */ + int getNumberOfChildren() { result = count(XMLElement e | xmlElements(e, _, this, _, _)) } - /** The number of places in the body of this XML parent where text occurs. */ - int getNumberOfCharacterSets() { - result = count(int pos | xmlChars(_,_,this,pos,_,_)) - } + /** Gets the number of places in the body of this XML parent where text occurs. */ + int getNumberOfCharacterSets() { result = count(int pos | xmlChars(_, _, this, pos, _, _)) } /** + * DEPRECATED: Internal. + * * Append the character sequences of this XML parent from left to right, separated by a space, * up to a specified (zero-based) index. */ - string charsSetUpTo(int n) { - (n = 0 and xmlChars(_,result,this,0,_,_)) or - (n > 0 and exists(string chars | xmlChars(_,chars,this,n,_,_) | - result = this.charsSetUpTo(n-1) + " " + chars)) - } - - /** Append all the character sequences of this XML parent from left to right, separated by a space. */ - string allCharactersString() { - exists(int n | n = this.getNumberOfCharacterSets() | - (n = 0 and result = "") or - (n > 0 and result = this.charsSetUpTo(n-1)) + deprecated string charsSetUpTo(int n) { + n = 0 and xmlChars(_, result, this, 0, _, _) + or + n > 0 and + exists(string chars | xmlChars(_, chars, this, n, _, _) | + result = this.charsSetUpTo(n - 1) + " " + chars ) } - /** The text value contained in this XML parent. */ - string getTextValue() { - result = allCharactersString() + /** + * Gets the result of appending all the character sequences of this XML parent from + * left to right, separated by a space. + */ + string allCharactersString() { + result = concat(string chars, int pos | + xmlChars(_, chars, this, pos, _, _) + | + chars, " " order by pos + ) } - /** A printable representation of this XML parent. */ + /** Gets the text value contained in this XML parent. */ + string getTextValue() { result = allCharactersString() } + + /** Gets a printable representation of this XML parent. */ string toString() { result = this.getName() } } /** An XML file. */ class XMLFile extends XMLParent, File { - XMLFile() { - xmlEncoding(this,_) - } + XMLFile() { xmlEncoding(this, _) } - /** A printable representation of this XML file. */ - override - string toString() { result = XMLParent.super.toString() } + /** Gets a printable representation of this XML file. */ + override string toString() { result = XMLParent.super.toString() } - /** The name of this XML file. */ - override - string getName() { files(this,result,_,_,_) } + /** Gets the name of this XML file. */ + override string getName() { result = File.super.getAbsolutePath() } - /** The path of this XML file. */ - string getPath() { files(this,_,result,_,_) } + /** + * DEPRECATED: Use `getAbsolutePath()` instead. + * + * Gets the path of this XML file. + */ + deprecated string getPath() { result = getAbsolutePath() } - /** The path of the folder that contains this XML file. */ - string getFolder() { - result = this.getPath().substring(0, this.getPath().length()-this.getName().length()) - } + /** + * DEPRECATED: Use `getParentContainer().getAbsolutePath()` instead. + * + * Gets the path of the folder that contains this XML file. + */ + deprecated string getFolder() { result = getParentContainer().getAbsolutePath() } - /** The encoding of this XML file. */ - string getEncoding() { xmlEncoding(this,result) } + /** Gets the encoding of this XML file. */ + string getEncoding() { xmlEncoding(this, result) } - /** The XML file itself. */ - override - XMLFile getFile() { result = this } + /** Gets the XML file itself. */ + override XMLFile getFile() { result = this } - /** A top-most element in an XML file. */ + /** Gets a top-most element in an XML file. */ XMLElement getARootElement() { result = this.getAChild() } - /** A DTD associated with this XML file. */ - XMLDTD getADTD() { xmlDTDs(result,_,_,_,this) } + /** Gets a DTD associated with this XML file. */ + XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) } } -/** A "Document Type Definition" of an XML file. */ +/** + * An XML document type definition (DTD). + * + * Example: + * + * ``` + * + * + * + * ``` + */ class XMLDTD extends @xmldtd { - /** The name of the root element of this DTD. */ - string getRoot() { xmlDTDs(this,result,_,_,_) } + /** Gets the name of the root element of this DTD. */ + string getRoot() { xmlDTDs(this, result, _, _, _) } - /** The public ID of this DTD. */ - string getPublicId() { xmlDTDs(this,_,result,_,_) } + /** Gets the public ID of this DTD. */ + string getPublicId() { xmlDTDs(this, _, result, _, _) } - /** The system ID of this DTD. */ - string getSystemId() { xmlDTDs(this,_,_,result,_) } + /** Gets the system ID of this DTD. */ + string getSystemId() { xmlDTDs(this, _, _, result, _) } - /** Whether this DTD is public. */ - predicate isPublic() { not xmlDTDs(this,_,"",_,_) } + /** Holds if this DTD is public. */ + predicate isPublic() { not xmlDTDs(this, _, "", _, _) } - /** The parent of this DTD. */ - XMLParent getParent() { xmlDTDs(this,_,_,_,result) } + /** Gets the parent of this DTD. */ + XMLParent getParent() { xmlDTDs(this, _, _, _, result) } - /** A printable representation of this DTD. */ + /** Gets a printable representation of this DTD. */ string toString() { - (this.isPublic() and result = this.getRoot() + " PUBLIC '" + - this.getPublicId() + "' '" + - this.getSystemId() + "'") or - (not this.isPublic() and result = this.getRoot() + - " SYSTEM '" + - this.getSystemId() + "'") + this.isPublic() and + result = this.getRoot() + " PUBLIC '" + this.getPublicId() + "' '" + this.getSystemId() + "'" + or + not this.isPublic() and + result = this.getRoot() + " SYSTEM '" + this.getSystemId() + "'" } } -/** An XML tag in an XML file. */ +/** + * An XML element in an XML file. + * + * Example: + * + * ``` + * + * + * ``` + */ class XMLElement extends @xmlelement, XMLParent, XMLLocatable { - /** Whether this XML element has the given `name`. */ + /** Holds if this XML element has the given `name`. */ predicate hasName(string name) { name = getName() } - /** The name of this XML element. */ - override - string getName() { xmlElements(this,result,_,_,_) } + /** Gets the name of this XML element. */ + override string getName() { xmlElements(this, result, _, _, _) } - /** The XML file in which this XML element occurs. */ - override - XMLFile getFile() { xmlElements(this,_,_,_,result) } + /** Gets the XML file in which this XML element occurs. */ + override XMLFile getFile() { xmlElements(this, _, _, _, result) } - /** The parent of this XML element. */ - XMLParent getParent() { xmlElements(this,_,result,_,_) } + /** Gets the parent of this XML element. */ + XMLParent getParent() { xmlElements(this, _, result, _, _) } - /** The index of this XML element among its parent's children. */ + /** Gets the index of this XML element among its parent's children. */ int getIndex() { xmlElements(this, _, _, result, _) } - /** Whether this XML element has a namespace. */ - predicate hasNamespace() { xmlHasNs(this,_,_) } + /** Holds if this XML element has a namespace. */ + predicate hasNamespace() { xmlHasNs(this, _, _) } - /** The namespace of this XML element, if any. */ - XMLNamespace getNamespace() { xmlHasNs(this,result,_) } + /** Gets the namespace of this XML element, if any. */ + XMLNamespace getNamespace() { xmlHasNs(this, result, _) } - /** The index of this XML element among its parent's children. */ - int getElementPositionIndex() { xmlElements(this,_,_,result,_) } + /** Gets the index of this XML element among its parent's children. */ + int getElementPositionIndex() { xmlElements(this, _, _, result, _) } - /** The depth of this element within the XML file tree structure. */ - override - int getDepth() { result = this.getParent().getDepth() + 1 } + /** Gets the depth of this element within the XML file tree structure. */ + override int getDepth() { result = this.getParent().getDepth() + 1 } - /** An XML attribute of this XML element. */ + /** Gets an XML attribute of this XML element. */ XMLAttribute getAnAttribute() { result.getElement() = this } - /** The attribute with the specified `name`, if any. */ - XMLAttribute getAttribute(string name) { - result.getElement() = this and result.getName() = name - } + /** Gets the attribute with the specified `name`, if any. */ + XMLAttribute getAttribute(string name) { result.getElement() = this and result.getName() = name } - /** Whether this XML element has an attribute with the specified `name`. */ - predicate hasAttribute(string name) { - exists(XMLAttribute a| a = this.getAttribute(name)) - } + /** Holds if this XML element has an attribute with the specified `name`. */ + predicate hasAttribute(string name) { exists(XMLAttribute a | a = this.getAttribute(name)) } - /** The value of the attribute with the specified `name`, if any. */ - string getAttributeValue(string name) { - result = this.getAttribute(name).getValue() - } + /** Gets the value of the attribute with the specified `name`, if any. */ + string getAttributeValue(string name) { result = this.getAttribute(name).getValue() } - /** A printable representation of this XML element. */ - override - string toString() { result = XMLParent.super.toString() } + /** Gets a printable representation of this XML element. */ + override string toString() { result = XMLParent.super.toString() } } -/** An attribute that occurs inside an XML element. */ +/** + * An attribute that occurs inside an XML element. + * + * Examples: + * + * ``` + * package="com.example.exampleapp" + * android:versionCode="1" + * ``` + */ class XMLAttribute extends @xmlattribute, XMLLocatable { - /** The name of this attribute. */ - string getName() { xmlAttrs(this,_,result,_,_,_) } + /** Gets the name of this attribute. */ + string getName() { xmlAttrs(this, _, result, _, _, _) } - /** The XML element to which this attribute belongs. */ - XMLElement getElement() { xmlAttrs(this,result,_,_,_,_) } + /** Gets the XML element to which this attribute belongs. */ + XMLElement getElement() { xmlAttrs(this, result, _, _, _, _) } - /** Whether this attribute has a namespace. */ - predicate hasNamespace() { xmlHasNs(this,_,_) } + /** Holds if this attribute has a namespace. */ + predicate hasNamespace() { xmlHasNs(this, _, _) } - /** The namespace of this attribute, if any. */ - XMLNamespace getNamespace() { xmlHasNs(this,result,_) } + /** Gets the namespace of this attribute, if any. */ + XMLNamespace getNamespace() { xmlHasNs(this, result, _) } - /** The value of this attribute. */ - string getValue() { xmlAttrs(this,_,_,result,_,_) } + /** Gets the value of this attribute. */ + string getValue() { xmlAttrs(this, _, _, result, _, _) } - /** A printable representation of this XML attribute. */ + /** Gets a printable representation of this XML attribute. */ override string toString() { result = this.getName() + "=" + this.getValue() } } -/** A namespace used in an XML file */ +/** + * A namespace used in an XML file. + * + * Example: + * + * ``` + * xmlns:android="http://schemas.android.com/apk/res/android" + * ``` + */ class XMLNamespace extends @xmlnamespace { - /** The prefix of this namespace. */ - string getPrefix() { xmlNs(this,result,_,_) } + /** Gets the prefix of this namespace. */ + string getPrefix() { xmlNs(this, result, _, _) } - /** The URI of this namespace. */ - string getURI() { xmlNs(this,_,result,_) } + /** Gets the URI of this namespace. */ + string getURI() { xmlNs(this, _, result, _) } - /** Whether this namespace has no prefix. */ + /** Holds if this namespace has no prefix. */ predicate isDefault() { this.getPrefix() = "" } - /** A printable representation of this XML namespace. */ + /** Gets a printable representation of this XML namespace. */ string toString() { - (this.isDefault() and result = this.getURI()) or - (not this.isDefault() and result = this.getPrefix() + ":" + this.getURI()) + this.isDefault() and result = this.getURI() + or + not this.isDefault() and result = this.getPrefix() + ":" + this.getURI() } } -/** A comment of the form `` is an XML comment. */ +/** + * A comment in an XML file. + * + * Example: + * + * ``` + * + * ``` + */ class XMLComment extends @xmlcomment, XMLLocatable { - /** The text content of this XML comment. */ - string getText() { xmlComments(this,result,_,_) } + /** Gets the text content of this XML comment. */ + string getText() { xmlComments(this, result, _, _) } - /** The parent of this XML comment. */ - XMLParent getParent() { xmlComments(this,_,result,_) } + /** Gets the parent of this XML comment. */ + XMLParent getParent() { xmlComments(this, _, result, _) } - /** A printable representation of this XML comment. */ + /** Gets a printable representation of this XML comment. */ override string toString() { result = this.getText() } } /** * A sequence of characters that occurs between opening and * closing tags of an XML element, excluding other elements. + * + * Example: + * + * ``` + * This is a sequence of characters. + * ``` */ class XMLCharacters extends @xmlcharacters, XMLLocatable { - /** The content of this character sequence. */ - string getCharacters() { xmlChars(this,result,_,_,_,_) } + /** Gets the content of this character sequence. */ + string getCharacters() { xmlChars(this, result, _, _, _, _) } - /** The parent of this character sequence. */ - XMLParent getParent() { xmlChars(this,_,result,_,_,_) } + /** Gets the parent of this character sequence. */ + XMLParent getParent() { xmlChars(this, _, result, _, _, _) } - /** Whether this character sequence is CDATA. */ - predicate isCDATA() { xmlChars(this,_,_,_,1,_) } + /** Holds if this character sequence is CDATA. */ + predicate isCDATA() { xmlChars(this, _, _, _, 1, _) } - /** A printable representation of this XML character sequence. */ + /** Gets a printable representation of this XML character sequence. */ override string toString() { result = this.getCharacters() } } diff --git a/python/ql/test/library-tests/web/twisted/Classes.expected b/python/ql/test/library-tests/web/twisted/Classes.expected new file mode 100644 index 00000000000..7713130d705 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Classes.expected @@ -0,0 +1,5 @@ +| class MyRequestHandler1 | test.py:3 | +| class MyRequestHandler2 | test.py:23 | +| class MyRequestHandler3 | test.py:27 | +| class MyRequestHandler4 | test.py:38 | +| class MyRequestHandler5 | test.py:42 | diff --git a/python/ql/test/library-tests/web/twisted/Classes.ql b/python/ql/test/library-tests/web/twisted/Classes.ql new file mode 100644 index 00000000000..ccc3618b61e --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Classes.ql @@ -0,0 +1,7 @@ +import python +import semmle.python.TestUtils +import semmle.python.web.twisted.Twisted + +from ClassValue cls +where cls = aTwistedRequestHandlerClass() +select cls.toString(), remove_library_prefix(cls.getScope().getLocation()) diff --git a/python/ql/test/library-tests/web/twisted/Methods.expected b/python/ql/test/library-tests/web/twisted/Methods.expected new file mode 100644 index 00000000000..e4ca6605ac9 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Methods.expected @@ -0,0 +1,8 @@ +| myrender | Function MyRequestHandler2.myrender | test.py:24 | +| render | Function MyRequestHandler1.render | test.py:4 | +| render | Function MyRequestHandler3.render | test.py:28 | +| render | Function MyRequestHandler4.render | test.py:39 | +| render | Function MyRequestHandler5.render | test.py:43 | +| render_GET | Function MyRequestHandler1.render_GET | test.py:9 | +| render_POST | Function MyRequestHandler1.render_POST | test.py:16 | +| render_POST | Function MyRequestHandler3.render_POST | test.py:31 | diff --git a/python/ql/test/library-tests/web/twisted/Methods.ql b/python/ql/test/library-tests/web/twisted/Methods.ql new file mode 100644 index 00000000000..f997b7deef3 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Methods.ql @@ -0,0 +1,7 @@ +import python +import semmle.python.TestUtils +import semmle.python.web.twisted.Twisted + +from FunctionValue func, string name +where func = getTwistedRequestHandlerMethod(name) +select name, func.toString(), remove_library_prefix(func.getScope().getLocation()) diff --git a/python/ql/test/library-tests/web/twisted/Sinks.expected b/python/ql/test/library-tests/web/twisted/Sinks.expected new file mode 100644 index 00000000000..f416a03e4b7 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Sinks.expected @@ -0,0 +1,8 @@ +| test.py:7 | response | externally controlled string | +| test.py:14 | response | externally controlled string | +| test.py:21 | response | externally controlled string | +| test.py:36 | do_stuff_with() | externally controlled string | +| test.py:40 | Str | externally controlled string | +| test.py:44 | Str | externally controlled string | +| test.py:45 | Str | externally controlled string | +| test.py:46 | Str | externally controlled string | diff --git a/python/ql/test/library-tests/web/twisted/Sinks.ql b/python/ql/test/library-tests/web/twisted/Sinks.ql new file mode 100644 index 00000000000..1045e9dda6b --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Sinks.ql @@ -0,0 +1,10 @@ +import python + +import semmle.python.web.HttpRequest +import semmle.python.web.HttpResponse +import semmle.python.security.strings.Untrusted +import semmle.python.TestUtils + +from TaintSink sink, TaintKind kind +where sink.sinks(kind) +select remove_library_prefix(sink.getLocation()), sink.(ControlFlowNode).getNode().toString(), kind diff --git a/python/ql/test/library-tests/web/twisted/Sources.expected b/python/ql/test/library-tests/web/twisted/Sources.expected new file mode 100644 index 00000000000..3015951d32b --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Sources.expected @@ -0,0 +1,8 @@ +| test.py:4 | request | twisted.request.http.Request | +| test.py:9 | request | twisted.request.http.Request | +| test.py:16 | request | twisted.request.http.Request | +| test.py:24 | request | twisted.request.http.Request | +| test.py:28 | myrequest | twisted.request.http.Request | +| test.py:31 | postrequest | twisted.request.http.Request | +| test.py:39 | request | twisted.request.http.Request | +| test.py:43 | request | twisted.request.http.Request | diff --git a/python/ql/test/library-tests/web/twisted/Sources.ql b/python/ql/test/library-tests/web/twisted/Sources.ql new file mode 100644 index 00000000000..bded6087ed2 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Sources.ql @@ -0,0 +1,11 @@ +import python +import semmle.python.TestUtils + +import semmle.python.web.HttpRequest +import semmle.python.web.HttpResponse +import semmle.python.security.strings.Untrusted + + +from TaintSource src, TaintKind kind +where src.isSourceOf(kind) +select remove_library_prefix(src.getLocation()), src.(ControlFlowNode).getNode().toString(), kind diff --git a/python/ql/test/library-tests/web/twisted/Taint.expected b/python/ql/test/library-tests/web/twisted/Taint.expected new file mode 100644 index 00000000000..1c793c973bd --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Taint.expected @@ -0,0 +1,41 @@ +| test.py:4 | request | twisted.request.http.Request | +| test.py:5 | Attribute | externally controlled string | +| test.py:5 | request | twisted.request.http.Request | +| test.py:6 | request | twisted.request.http.Request | +| test.py:9 | request | twisted.request.http.Request | +| test.py:10 | request | twisted.request.http.Request | +| test.py:11 | Attribute | externally controlled string | +| test.py:11 | x | twisted.request.http.Request | +| test.py:12 | request | twisted.request.http.Request | +| test.py:13 | request | twisted.request.http.Request | +| test.py:16 | request | twisted.request.http.Request | +| test.py:17 | Attribute | {[externally controlled string]} | +| test.py:17 | request | twisted.request.http.Request | +| test.py:18 | Attribute | {[externally controlled string]} | +| test.py:18 | Attribute() | [externally controlled string] | +| test.py:18 | request | twisted.request.http.Request | +| test.py:19 | Subscript | externally controlled string | +| test.py:19 | foo | [externally controlled string] | +| test.py:20 | quux | externally controlled string | +| test.py:24 | request | twisted.request.http.Request | +| test.py:25 | request | twisted.request.http.Request | +| test.py:28 | myrequest | twisted.request.http.Request | +| test.py:29 | myrequest | twisted.request.http.Request | +| test.py:31 | postrequest | twisted.request.http.Request | +| test.py:32 | Attribute() | externally controlled string | +| test.py:32 | postrequest | twisted.request.http.Request | +| test.py:33 | Attribute() | externally controlled string | +| test.py:33 | postrequest | twisted.request.http.Request | +| test.py:34 | Attribute() | externally controlled string | +| test.py:34 | postrequest | twisted.request.http.Request | +| test.py:35 | Attribute() | externally controlled string | +| test.py:35 | postrequest | twisted.request.http.Request | +| test.py:36 | w | externally controlled string | +| test.py:36 | x | externally controlled string | +| test.py:36 | y | externally controlled string | +| test.py:36 | z | externally controlled string | +| test.py:39 | request | twisted.request.http.Request | +| test.py:40 | request | twisted.request.http.Request | +| test.py:43 | request | twisted.request.http.Request | +| test.py:44 | request | twisted.request.http.Request | +| test.py:45 | request | twisted.request.http.Request | diff --git a/python/ql/test/library-tests/web/twisted/Taint.ql b/python/ql/test/library-tests/web/twisted/Taint.ql new file mode 100644 index 00000000000..f0a95299113 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/Taint.ql @@ -0,0 +1,11 @@ +import python +import semmle.python.TestUtils + +import semmle.python.web.HttpRequest +import semmle.python.web.HttpResponse +import semmle.python.security.strings.Untrusted + +from TaintedNode node + +select remove_library_prefix(node.getLocation()), node.getAstNode().toString(), node.getTaintKind() + diff --git a/python/ql/test/library-tests/web/twisted/options b/python/ql/test/library-tests/web/twisted/options new file mode 100644 index 00000000000..6979a743e58 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/options @@ -0,0 +1,2 @@ +semmle-extractor-options: --max-import-depth=1 -p ../../../query-tests/Security/lib/ +optimize: true diff --git a/python/ql/test/library-tests/web/twisted/test.py b/python/ql/test/library-tests/web/twisted/test.py new file mode 100644 index 00000000000..f6691b84d95 --- /dev/null +++ b/python/ql/test/library-tests/web/twisted/test.py @@ -0,0 +1,51 @@ +from twisted.web import resource + +class MyRequestHandler1(resource.Resource): + def render(self, request): + foo(request.uri) + response = do_stuff_with(request) + return response + + def render_GET(self, request): + x = request + bar(x.uri) + do_stuff_with(request) + response = do_stuff_with(request) + return response + + def render_POST(self, request): + baz(request.args) + foo = request.args.get("baz") + quux = foo[5] + response = do_stuff_with(quux) + return response + +class MyRequestHandler2(resource.Resource): + def myrender(self, request): + do_stuff_with(request) + +class MyRequestHandler3(resource.Resource): + def render(self, myrequest): + do_stuff_with(myrequest) + + def render_POST(self, postrequest): + x = postrequest.getHeader("someheader") + y = postrequest.getCookie("somecookie") + z = postrequest.getUser() + w = postrequest.getPassword() + return do_stuff_with(x,y,z,w) + +class MyRequestHandler4(resource.Resource): + def render(self, request): + request.write("Foobar") + +class MyRequestHandler5(resource.Resource): + def render(self, request): + request.setHeader("foo", "bar") + request.addCookie("key", "value") + return "This is my response." + +class NotATwistedRequestHandler(object): + def render(self, request): + return do_stuff_with(request) + diff --git a/python/ql/test/library-tests/web/zope/Test.expected b/python/ql/test/library-tests/web/zope/Test.expected new file mode 100644 index 00000000000..bb3ca1f441e --- /dev/null +++ b/python/ql/test/library-tests/web/zope/Test.expected @@ -0,0 +1,3 @@ +| 12 | ControlFlowNode for implementer | class implementer | ../../../query-tests/Security/lib/zope/interface/__init__.py:5 | +| 13 | ControlFlowNode for IThing | class IThing | test.py:4 | +| 14 | ControlFlowNode for Thing | class Thing | test.py:9 | diff --git a/python/ql/test/library-tests/web/zope/Test.ql b/python/ql/test/library-tests/web/zope/Test.ql new file mode 100644 index 00000000000..e694883237b --- /dev/null +++ b/python/ql/test/library-tests/web/zope/Test.ql @@ -0,0 +1,10 @@ +import python +import semmle.python.TestUtils + +from ControlFlowNode f, Value v, ControlFlowNode x +where + exists(ExprStmt s | s.getValue().getAFlowNode() = f) and + f.pointsTo(v, x) and + f.getLocation().getFile().getBaseName() = "test.py" +select f.getLocation().getStartLine(), f.toString(), v.toString(), + remove_library_prefix(x.getLocation()) diff --git a/python/ql/test/library-tests/web/zope/options b/python/ql/test/library-tests/web/zope/options new file mode 100644 index 00000000000..7fb713d5924 --- /dev/null +++ b/python/ql/test/library-tests/web/zope/options @@ -0,0 +1 @@ +semmle-extractor-options: --max-import-depth=3 -p ../../../query-tests/Security/lib/ diff --git a/python/ql/test/library-tests/web/zope/test.py b/python/ql/test/library-tests/web/zope/test.py new file mode 100644 index 00000000000..64ac9138d7e --- /dev/null +++ b/python/ql/test/library-tests/web/zope/test.py @@ -0,0 +1,14 @@ + +from zope.interface import Interface, implementer + +class IThing(Interface): + pass + + +@implementer(IThing) +class Thing(object): + pass + +implementer +IThing +Thing diff --git a/python/ql/test/query-tests/Security/lib/twisted/__init__.py b/python/ql/test/query-tests/Security/lib/twisted/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/test/query-tests/Security/lib/twisted/web/__init__.py b/python/ql/test/query-tests/Security/lib/twisted/web/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/test/query-tests/Security/lib/twisted/web/http.py b/python/ql/test/query-tests/Security/lib/twisted/web/http.py new file mode 100644 index 00000000000..c7ac65eaf05 --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/twisted/web/http.py @@ -0,0 +1,2 @@ +class Request(object): + pass diff --git a/python/ql/test/query-tests/Security/lib/twisted/web/resource.py b/python/ql/test/query-tests/Security/lib/twisted/web/resource.py new file mode 100644 index 00000000000..1fe186864cb --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/twisted/web/resource.py @@ -0,0 +1,2 @@ +class Resource(object): + pass diff --git a/python/ql/test/query-tests/Security/lib/zope/__init__.py b/python/ql/test/query-tests/Security/lib/zope/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/test/query-tests/Security/lib/zope/interface/__init__.py b/python/ql/test/query-tests/Security/lib/zope/interface/__init__.py new file mode 100644 index 00000000000..423c7b58341 --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/zope/interface/__init__.py @@ -0,0 +1,9 @@ +class Interface(): + pass + + +class implementer: + + def __call__(self, ob): + ... + return ob