diff --git a/change-notes/1.24/analysis-java.md b/change-notes/1.24/analysis-java.md index 0598b813415..36210c0457e 100644 --- a/change-notes/1.24/analysis-java.md +++ b/change-notes/1.24/analysis-java.md @@ -5,6 +5,7 @@ The following changes in version 1.24 affect Java analysis in all applications. ## General improvements * Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`). +* A `Customizations.qll` file has been added to allow customizations of the standard library that apply to all queries. ## New queries diff --git a/change-notes/1.24/analysis-javascript.md b/change-notes/1.24/analysis-javascript.md index bea62aa9e85..b17ddfb4860 100644 --- a/change-notes/1.24/analysis-javascript.md +++ b/change-notes/1.24/analysis-javascript.md @@ -46,6 +46,8 @@ | Polynomial regular expression used on uncontrolled data (`js/polynomial-redos`) | security, external/cwe/cwe-730, external/cwe/cwe-400 | Highlights expensive regular expressions that may be used on malicious input. Results are shown on LGTM by default. | | Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive copying operations that are susceptible to prototype pollution. Results are shown on LGTM by default. | | Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | Highlights potential XSS vulnerabilities in unsafely designed jQuery plugins. Results are shown on LGTM by default. | +| Unnecessary use of `cat` process (`js/unnecessary-use-of-cat`) | correctness, security, maintainability | Highlights command executions of `cat` where the fs API should be used instead. Results are shown on LGTM by default. | + ## Changes to existing queries diff --git a/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll b/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll index 4ca68fb27c4..e0cc92b6ce8 100644 --- a/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll +++ b/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll @@ -15,24 +15,33 @@ class ConstantZero extends Expr { } } +/** + * Holds if `candidate` is an expression such that if it's unsigned then we + * want an alert at `ge`. + */ +private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) { + // Base case: `candidate >= 0` + ge.getRightOperand() instanceof ConstantZero and + candidate = ge.getLeftOperand().getFullyConverted() and + // left operand was a signed or unsigned IntegralType before conversions + // (not a pointer, checking a pointer >= 0 is an entirely different mistake) + // (not an enum, as the fully converted type of an enum is compiler dependent + // so checking an enum >= 0 is always reasonable) + ge.getLeftOperand().getUnderlyingType() instanceof IntegralType + or + // Recursive case: `...(largerType)candidate >= 0` + exists(Conversion conversion | + lookForUnsignedAt(ge, conversion) and + candidate = conversion.getExpr() and + conversion.getType().getSize() > candidate.getType().getSize() + ) +} + class UnsignedGEZero extends GEExpr { UnsignedGEZero() { - this.getRightOperand() instanceof ConstantZero and - // left operand was a signed or unsigned IntegralType before conversions - // (not a pointer, checking a pointer >= 0 is an entirely different mistake) - // (not an enum, as the fully converted type of an enum is compiler dependent - // so checking an enum >= 0 is always reasonable) - getLeftOperand().getUnderlyingType() instanceof IntegralType and exists(Expr ue | - // ue is some conversion of the left operand - ue = getLeftOperand().getConversion*() and - // ue is unsigned - ue.getUnderlyingType().(IntegralType).isUnsigned() and - // ue may be converted to zero or more strictly larger possibly signed types - // before it is fully converted - forall(Expr following | following = ue.getConversion+() | - following.getType().getSize() > ue.getType().getSize() - ) + lookForUnsignedAt(this, ue) and + ue.getUnderlyingType().(IntegralType).isUnsigned() ) } } diff --git a/cpp/ql/src/semmle/code/cpp/Field.qll b/cpp/ql/src/semmle/code/cpp/Field.qll index de52fd79788..eb864a16063 100644 --- a/cpp/ql/src/semmle/code/cpp/Field.qll +++ b/cpp/ql/src/semmle/code/cpp/Field.qll @@ -19,6 +19,8 @@ import semmle.code.cpp.exprs.Access class Field extends MemberVariable { Field() { fieldoffsets(underlyingElement(this), _, _) } + override string getCanonicalQLClass() { result = "Field" } + /** * Gets the offset of this field in bytes from the start of its declaring * type (on the machine where facts were extracted). @@ -84,6 +86,8 @@ class Field extends MemberVariable { class BitField extends Field { BitField() { bitfield(underlyingElement(this), _, _) } + override string getCanonicalQLClass() { result = "BitField" } + /** * Gets the size of this bitfield in bits (on the machine where facts * were extracted). diff --git a/cpp/ql/src/semmle/code/cpp/Variable.qll b/cpp/ql/src/semmle/code/cpp/Variable.qll index cf85f1f9fd2..8e4a5d73664 100644 --- a/cpp/ql/src/semmle/code/cpp/Variable.qll +++ b/cpp/ql/src/semmle/code/cpp/Variable.qll @@ -28,6 +28,8 @@ private import semmle.code.cpp.internal.ResolveClass * can have multiple declarations. */ class Variable extends Declaration, @variable { + override string getCanonicalQLClass() { result = "Variable" } + /** Gets the initializer of this variable, if any. */ Initializer getInitializer() { result.getDeclaration() = this } @@ -351,6 +353,8 @@ class StackVariable extends LocalScopeVariable { * A local variable can be declared by a `DeclStmt` or a `ConditionDeclExpr`. */ class LocalVariable extends LocalScopeVariable, @localvariable { + override string getCanonicalQLClass() { result = "LocalVariable" } + override string getName() { localvariables(underlyingElement(this), _, result) } override Type getType() { localvariables(underlyingElement(this), unresolveElement(result), _) } @@ -396,6 +400,8 @@ class NamespaceVariable extends GlobalOrNamespaceVariable { NamespaceVariable() { exists(Namespace n | namespacembrs(unresolveElement(n), underlyingElement(this))) } + + override string getCanonicalQLClass() { result = "NamespaceVariable" } } /** @@ -415,6 +421,8 @@ class NamespaceVariable extends GlobalOrNamespaceVariable { */ class GlobalVariable extends GlobalOrNamespaceVariable { GlobalVariable() { not this instanceof NamespaceVariable } + + override string getCanonicalQLClass() { result = "GlobalVariable" } } /** @@ -434,6 +442,8 @@ class GlobalVariable extends GlobalOrNamespaceVariable { class MemberVariable extends Variable, @membervariable { MemberVariable() { this.isMember() } + override string getCanonicalQLClass() { result = "MemberVariable" } + /** Holds if this member is private. */ predicate isPrivate() { this.hasSpecifier("private") } diff --git a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll index c71403e84a2..079b031c530 100644 --- a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll +++ b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll @@ -442,6 +442,20 @@ class Expr extends StmtParent, @expr { else result = this } + /** + * Gets the unique non-`Conversion` expression `e` for which + * `this = e.getConversion*()`. + * + * For example, if called on the expression `(int)(char)x`, this predicate + * gets the expression `x`. + */ + Expr getUnconverted() { + not this instanceof Conversion and + result = this + or + result = this.(Conversion).getExpr().getUnconverted() + } + /** * Gets the type of this expression, after any implicit conversions and explicit casts, and after resolving typedefs. * 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 638e0e0269c..4312afa4611 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll @@ -267,7 +267,7 @@ private predicate modelTaintToParameter(Function f, int parameterIn, int paramet /** * Holds if `chi` is on the chain of chi-instructions for all aliased memory. - * Taint shoud not pass through these instructions since they tend to mix up + * Taint should not pass through these instructions since they tend to mix up * unrelated objects. */ private predicate isChiForAllAliasedMemory(Instruction instr) { 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 b10997f3f18..e19912a63ee 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 @@ -131,10 +131,7 @@ class ExprNode extends InstructionNode { * `Conversion`, then the result is that `Conversion`'s non-`Conversion` base * expression. */ - Expr getExpr() { - result.getConversion*() = instr.getConvertedResultExpression() and - not result instanceof Conversion - } + Expr getExpr() { result = instr.getUnconvertedResultExpression() } /** * Gets the expression corresponding to this node, if any. The returned diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll index 886183c32ba..83c9ac1215c 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll @@ -82,6 +82,7 @@ private newtype TOpcode = TSizedBufferReadSideEffect() or TSizedBufferMustWriteSideEffect() or TSizedBufferMayWriteSideEffect() or + TInitializeDynamicAllocation() or TChi() or TInlineAsm() or TUnreached() or @@ -213,23 +214,28 @@ abstract class IndirectReadOpcode extends IndirectMemoryAccessOpcode { } /** - * An opcode that accesses a memory buffer of unknown size. + * An opcode that accesses a memory buffer. */ abstract class BufferAccessOpcode extends Opcode { final override predicate hasAddressOperand() { any() } } +/** + * An opcode that accesses a memory buffer of unknown size. + */ +abstract class UnsizedBufferAccessOpcode extends BufferAccessOpcode { } + /** * An opcode that writes to a memory buffer of unknown size. */ -abstract class BufferWriteOpcode extends BufferAccessOpcode { +abstract class UnsizedBufferWriteOpcode extends UnsizedBufferAccessOpcode { final override MemoryAccessKind getWriteMemoryAccess() { result instanceof BufferMemoryAccess } } /** * An opcode that reads from a memory buffer of unknown size. */ -abstract class BufferReadOpcode extends BufferAccessOpcode { +abstract class UnsizedBufferReadOpcode extends UnsizedBufferAccessOpcode { final override MemoryAccessKind getReadMemoryAccess() { result instanceof BufferMemoryAccess } } @@ -261,9 +267,7 @@ abstract class EntireAllocationReadOpcode extends EntireAllocationAccessOpcode { /** * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`. */ -abstract class SizedBufferAccessOpcode extends Opcode { - final override predicate hasAddressOperand() { any() } - +abstract class SizedBufferAccessOpcode extends BufferAccessOpcode { final override predicate hasBufferSizeOperand() { any() } } @@ -666,17 +670,18 @@ module Opcode { final override string toString() { result = "IndirectMayWriteSideEffect" } } - class BufferReadSideEffect extends ReadSideEffectOpcode, BufferReadOpcode, TBufferReadSideEffect { + class BufferReadSideEffect extends ReadSideEffectOpcode, UnsizedBufferReadOpcode, + TBufferReadSideEffect { final override string toString() { result = "BufferReadSideEffect" } } - class BufferMustWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, + class BufferMustWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, TBufferMustWriteSideEffect { final override string toString() { result = "BufferMustWriteSideEffect" } } - class BufferMayWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, MayWriteOpcode, - TBufferMayWriteSideEffect { + class BufferMayWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, + MayWriteOpcode, TBufferMayWriteSideEffect { final override string toString() { result = "BufferMayWriteSideEffect" } } @@ -695,6 +700,11 @@ module Opcode { final override string toString() { result = "SizedBufferMayWriteSideEffect" } } + class InitializeDynamicAllocation extends SideEffectOpcode, EntireAllocationWriteOpcode, + TInitializeDynamicAllocation { + final override string toString() { result = "InitializeDynamicAllocation" } + } + class Chi extends Opcode, TChi { final override string toString() { result = "Chi" } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll index ee7aebc4749..ae585ec2c7e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll @@ -176,7 +176,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { /** * A temporary variable introduced by IR construction. The most common examples are the variable - * generated to hold the return value of afunction, or the variable generated to hold the result of + * generated to hold the return value of a function, or the variable generated to hold the result of * a condition operator (`a ? b : c`). */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { 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 d058a6c5a3e..d14cdf08910 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 @@ -1350,6 +1350,26 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } +/** + * An instruction representing the initial value of newly allocated memory, e.g. the result of a + * call to `malloc`. + */ +class InitializeDynamicAllocationInstruction extends SideEffectInstruction { + InitializeDynamicAllocationInstruction() { + getOpcode() instanceof Opcode::InitializeDynamicAllocation + } + + /** + * Gets the address of the allocation this instruction is initializing. + */ + final AddressOperand getAllocationAddressOperand() { result = getAnOperand() } + + /** + * Gets the operand for the allocation this instruction is initializing. + */ + final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() } +} + /** * An instruction representing a GNU or MSVC inline assembly statement. */ diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll index d9937294d70..e95086c89fc 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll @@ -7,6 +7,9 @@ private newtype TAllocation = TVariableAllocation(IRVariable var) or TIndirectParameterAllocation(IRAutomaticUserVariable var) { exists(InitializeIndirectionInstruction instr | instr.getIRVariable() = var) + } or + TDynamicAllocation(CallInstruction call) { + exists(InitializeDynamicAllocationInstruction instr | instr.getPrimaryInstruction() = call) } /** @@ -95,3 +98,29 @@ class IndirectParameterAllocation extends Allocation, TIndirectParameterAllocati final override predicate alwaysEscapes() { none() } } + +class DynamicAllocation extends Allocation, TDynamicAllocation { + CallInstruction call; + + DynamicAllocation() { this = TDynamicAllocation(call) } + + final override string toString() { + result = call.toString() + " at " + call.getLocation() // This isn't performant, but it's only used in test/dump code right now. + } + + final override CallInstruction getABaseInstruction() { result = call } + + final override IRFunction getEnclosingIRFunction() { result = call.getEnclosingIRFunction() } + + final override Language::Location getLocation() { result = call.getLocation() } + + final override string getUniqueId() { result = call.getUniqueId() } + + final override IRType getIRType() { result instanceof IRUnknownType } + + final override predicate isReadOnly() { none() } + + final override predicate isAlwaysAllocatedOnStack() { none() } + + final override predicate alwaysEscapes() { none() } +} diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll index c65570c5999..e754e8d3849 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll @@ -26,7 +26,7 @@ private predicate hasResultMemoryAccess( type = languageType.getIRType() and isIndirectOrBufferMemoryAccess(instr.getResultMemoryAccess()) and (if instr.hasResultMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and - if type.getByteSize() > 0 + if exists(type.getByteSize()) then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8)) else endBitOffset = Ints::unknown() ) @@ -43,7 +43,7 @@ private predicate hasOperandMemoryAccess( type = languageType.getIRType() and isIndirectOrBufferMemoryAccess(operand.getMemoryAccess()) and (if operand.hasMayReadMemoryAccess() then isMayAccess = true else isMayAccess = false) and - if type.getByteSize() > 0 + if exists(type.getByteSize()) then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8)) else endBitOffset = Ints::unknown() ) @@ -68,8 +68,12 @@ private newtype TMemoryLocation = ) and languageType = type.getCanonicalLanguageType() } or - TEntireAllocationMemoryLocation(IndirectParameterAllocation var, boolean isMayAccess) { - isMayAccess = false or isMayAccess = true + TEntireAllocationMemoryLocation(Allocation var, boolean isMayAccess) { + ( + var instanceof IndirectParameterAllocation or + var instanceof DynamicAllocation + ) and + (isMayAccess = false or isMayAccess = true) } or TUnknownMemoryLocation(IRFunction irFunc, boolean isMayAccess) { isMayAccess = false or isMayAccess = true diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll index ee7aebc4749..ae585ec2c7e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll @@ -176,7 +176,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { /** * A temporary variable introduced by IR construction. The most common examples are the variable - * generated to hold the return value of afunction, or the variable generated to hold the result of + * generated to hold the return value of a function, or the variable generated to hold the result of * a condition operator (`a ? b : c`). */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { 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 d058a6c5a3e..d14cdf08910 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 @@ -1350,6 +1350,26 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } +/** + * An instruction representing the initial value of newly allocated memory, e.g. the result of a + * call to `malloc`. + */ +class InitializeDynamicAllocationInstruction extends SideEffectInstruction { + InitializeDynamicAllocationInstruction() { + getOpcode() instanceof Opcode::InitializeDynamicAllocation + } + + /** + * Gets the address of the allocation this instruction is initializing. + */ + final AddressOperand getAllocationAddressOperand() { result = getAnOperand() } + + /** + * Gets the operand for the allocation this instruction is initializing. + */ + final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() } +} + /** * An instruction representing a GNU or MSVC inline assembly statement. */ 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 96ae480405a..dbb27cd1bca 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 @@ -68,14 +68,7 @@ private module Cached { cached Expr getInstructionUnconvertedResultExpression(Instruction instruction) { - exists(Expr converted | - result = converted.(Conversion).getExpr+() - or - result = converted - | - not result instanceof Conversion and - converted = getInstructionConvertedResultExpression(instruction) - ) + result = getInstructionConvertedResultExpression(instruction).getUnconverted() } cached diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll index 93e5c43bf3b..fb7dc173812 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll @@ -341,16 +341,32 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects { ) } - override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) { none() } + override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) { + expr.getTarget() instanceof AllocationFunction and + opcode instanceof Opcode::InitializeDynamicAllocation and + tag = OnlyInstructionTag() and + type = getUnknownType() + } - override Instruction getFirstInstruction() { result = getChild(0).getFirstInstruction() } + override Instruction getFirstInstruction() { + if expr.getTarget() instanceof AllocationFunction + then result = getInstruction(OnlyInstructionTag()) + else result = getChild(0).getFirstInstruction() + } - override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() } + override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { + tag = OnlyInstructionTag() and + kind = gotoEdge() and + expr.getTarget() instanceof AllocationFunction and + if exists(getChild(0)) + then result = getChild(0).getFirstInstruction() + else result = getParent().getChildSuccessor(this) + } - override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) { none() } - - override CppType getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) { - none() + override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) { + tag = OnlyInstructionTag() and + operandTag = addressOperand() and + result = getPrimaryInstructionForSideEffect(OnlyInstructionTag()) } override Instruction getPrimaryInstructionForSideEffect(InstructionTag tag) { @@ -487,7 +503,7 @@ class TranslatedSideEffect extends TranslatedElement, TTranslatedArgumentSideEff } override CppType getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) { - if hasSpecificReadSideEffect(any(Opcode::BufferReadSideEffect op)) + if hasSpecificReadSideEffect(any(BufferAccessOpcode op)) then result = getUnknownType() and tag instanceof OnlyInstructionTag and diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll index 64dc7e869c2..81abc58495d 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll @@ -411,7 +411,9 @@ newtype TTranslatedElement = TTranslatedConditionDecl(ConditionDeclExpr expr) { not ignoreExpr(expr) } or // The side effects of a `Call` TTranslatedSideEffects(Call expr) { - exists(TTranslatedArgumentSideEffect(expr, _, _, _)) or expr instanceof ConstructorCall + exists(TTranslatedArgumentSideEffect(expr, _, _, _)) or + expr instanceof ConstructorCall or + expr.getTarget() instanceof AllocationFunction } or // A precise side effect of an argument to a `Call` TTranslatedArgumentSideEffect(Call call, Expr expr, int n, boolean isWrite) { ( diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll index ee7aebc4749..ae585ec2c7e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll @@ -176,7 +176,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { /** * A temporary variable introduced by IR construction. The most common examples are the variable - * generated to hold the return value of afunction, or the variable generated to hold the result of + * generated to hold the return value of a function, or the variable generated to hold the result of * a condition operator (`a ? b : c`). */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { 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 d058a6c5a3e..d14cdf08910 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 @@ -1350,6 +1350,26 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } +/** + * An instruction representing the initial value of newly allocated memory, e.g. the result of a + * call to `malloc`. + */ +class InitializeDynamicAllocationInstruction extends SideEffectInstruction { + InitializeDynamicAllocationInstruction() { + getOpcode() instanceof Opcode::InitializeDynamicAllocation + } + + /** + * Gets the address of the allocation this instruction is initializing. + */ + final AddressOperand getAllocationAddressOperand() { result = getAnOperand() } + + /** + * Gets the operand for the allocation this instruction is initializing. + */ + final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() } +} + /** * An instruction representing a GNU or MSVC inline assembly statement. */ diff --git a/cpp/ql/src/semmle/code/cpp/stmts/Stmt.qll b/cpp/ql/src/semmle/code/cpp/stmts/Stmt.qll index f64b5f2fb31..1640bee0f35 100644 --- a/cpp/ql/src/semmle/code/cpp/stmts/Stmt.qll +++ b/cpp/ql/src/semmle/code/cpp/stmts/Stmt.qll @@ -1604,30 +1604,18 @@ class EnumSwitch extends SwitchStmt { * ``` * there are results `GREEN` and `BLUE`. */ - pragma[noopt] EnumConstant getAMissingCase() { exists(Enum et | - exists(Expr e, Type t | - e = this.getExpr() and - this instanceof EnumSwitch and - t = e.getType() and - et = t.getUnderlyingType() - ) and + et = this.getExpr().getUnderlyingType() and result = et.getAnEnumConstant() and - not exists(string value | - exists(SwitchCase sc, Expr e | - sc = this.getASwitchCase() and - e = sc.getExpr() and - value = e.getValue() - ) and - exists(Initializer init, Expr e | - init = result.getInitializer() and - e = init.getExpr() and - e.getValue() = value - ) - ) + not this.matchesValue(result.getInitializer().getExpr().getValue()) ) } + + pragma[noinline] + private predicate matchesValue(string value) { + value = this.getASwitchCase().getExpr().getValue() + } } /** diff --git a/cpp/ql/test/library-tests/dataflow/security-taint/tainted.expected b/cpp/ql/test/library-tests/dataflow/security-taint/tainted.expected index 7359b068d8e..a62b9b3e204 100644 --- a/cpp/ql/test/library-tests/dataflow/security-taint/tainted.expected +++ b/cpp/ql/test/library-tests/dataflow/security-taint/tainted.expected @@ -52,3 +52,18 @@ | test.cpp:75:20:75:25 | call to getenv | test.cpp:75:15:75:18 | call to atoi | | | test.cpp:75:20:75:25 | call to getenv | test.cpp:75:20:75:25 | call to getenv | | | test.cpp:75:20:75:25 | call to getenv | test.cpp:75:20:75:45 | (const char *)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:8:24:8:25 | s1 | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:11:20:11:21 | s1 | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:11:36:11:37 | s2 | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:83:17:83:24 | userName | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:83:28:83:33 | call to getenv | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:83:28:83:46 | (const char *)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:85:8:85:11 | copy | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:86:2:86:7 | call to strcpy | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:86:9:86:12 | copy | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:86:15:86:22 | userName | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:6:88:27 | ! ... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:7:88:12 | call to strcmp | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:7:88:27 | (bool)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:14:88:17 | (const char *)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:14:88:17 | copy | | diff --git a/cpp/ql/test/library-tests/dataflow/security-taint/tainted_diff.expected b/cpp/ql/test/library-tests/dataflow/security-taint/tainted_diff.expected index ed954b5444a..ca905d71823 100644 --- a/cpp/ql/test/library-tests/dataflow/security-taint/tainted_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/security-taint/tainted_diff.expected @@ -8,3 +8,6 @@ | test.cpp:68:28:68:33 | call to getenv | test.cpp:69:10:69:13 | copy | AST only | | test.cpp:68:28:68:33 | call to getenv | test.cpp:70:12:70:15 | copy | AST only | | test.cpp:68:28:68:33 | call to getenv | test.cpp:71:12:71:15 | copy | AST only | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:11:20:11:21 | s1 | AST only | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:85:8:85:11 | copy | AST only | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:86:9:86:12 | copy | AST only | diff --git a/cpp/ql/test/library-tests/dataflow/security-taint/tainted_ir.expected b/cpp/ql/test/library-tests/dataflow/security-taint/tainted_ir.expected index 114c213ff54..1a7027d399c 100644 --- a/cpp/ql/test/library-tests/dataflow/security-taint/tainted_ir.expected +++ b/cpp/ql/test/library-tests/dataflow/security-taint/tainted_ir.expected @@ -40,3 +40,15 @@ | test.cpp:75:20:75:25 | call to getenv | test.cpp:75:15:75:18 | call to atoi | | | test.cpp:75:20:75:25 | call to getenv | test.cpp:75:20:75:25 | call to getenv | | | test.cpp:75:20:75:25 | call to getenv | test.cpp:75:20:75:45 | (const char *)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:8:24:8:25 | s1 | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:11:36:11:37 | s2 | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:83:17:83:24 | userName | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:83:28:83:33 | call to getenv | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:83:28:83:46 | (const char *)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:86:2:86:7 | call to strcpy | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:86:15:86:22 | userName | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:6:88:27 | ! ... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:7:88:12 | call to strcmp | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:7:88:27 | (bool)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:14:88:17 | (const char *)... | | +| test.cpp:83:28:83:33 | call to getenv | test.cpp:88:14:88:17 | copy | | diff --git a/cpp/ql/test/library-tests/dataflow/security-taint/test.cpp b/cpp/ql/test/library-tests/dataflow/security-taint/test.cpp index 5eb7ee79b46..9d28f9680ca 100644 --- a/cpp/ql/test/library-tests/dataflow/security-taint/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/security-taint/test.cpp @@ -76,3 +76,16 @@ void guard() { if (len > 1000) return; char **node = (char **) malloc(len * sizeof(char *)); } + +const char *alias_global; + +void mallocBuffer() { + const char *userName = getenv("USER_NAME"); + char *alias = (char*)malloc(4096); + char *copy = (char*)malloc(4096); + strcpy(copy, userName); + alias_global = alias; // to force a Chi node on all aliased memory + if (!strcmp(copy, "admin")) { // copy should be tainted + isAdmin = true; + } +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected index 912fa5bf1b3..76f564d1f7b 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected @@ -27,3 +27,5 @@ | declarationEntry.cpp:31:4:31:19 | myMemberVariable | declarationEntry.cpp:31:4:31:19 | definition of myMemberVariable | 1 | 1 | | declarationEntry.cpp:34:22:34:28 | mtc_int | declarationEntry.cpp:34:22:34:28 | definition of mtc_int | 1 | 1 | | declarationEntry.cpp:35:24:35:32 | mtc_short | declarationEntry.cpp:35:24:35:32 | definition of mtc_short | 1 | 1 | +| macro.c:2:1:2:3 | foo | macro.c:2:1:2:3 | declaration of foo | 1 | 1 | +| macro.c:4:5:4:8 | main | macro.c:4:5:4:8 | definition of main | 1 | 1 | diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected index 6a452473c72..9a544200cae 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected @@ -1,12 +1,14 @@ -| declarationEntry.c:2:6:2:20 | declaration of myFirstFunction | | -| declarationEntry.c:4:6:4:21 | definition of mySecondFunction | | -| declarationEntry.c:8:6:8:20 | definition of myThirdFunction | | -| declarationEntry.c:13:2:13:2 | declaration of myFourthFunction | isImplicit | -| declarationEntry.c:14:2:14:2 | declaration of myFifthFunction | isImplicit | -| declarationEntry.c:17:6:17:21 | declaration of myFourthFunction | | -| declarationEntry.cpp:9:6:9:15 | declaration of myFunction | | -| declarationEntry.cpp:11:6:11:15 | definition of myFunction | | -| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | -| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | -| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | -| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | +| declarationEntry.c:2:6:2:20 | declaration of myFirstFunction | | 1 | c_linkage | +| declarationEntry.c:4:6:4:21 | definition of mySecondFunction | | 1 | c_linkage | +| declarationEntry.c:8:6:8:20 | definition of myThirdFunction | | 1 | c_linkage | +| declarationEntry.c:13:2:13:2 | declaration of myFourthFunction | isImplicit | 1 | c_linkage | +| declarationEntry.c:14:2:14:2 | declaration of myFifthFunction | isImplicit | 1 | c_linkage | +| declarationEntry.c:17:6:17:21 | declaration of myFourthFunction | | 1 | c_linkage | +| declarationEntry.cpp:9:6:9:15 | declaration of myFunction | | 0 | | +| declarationEntry.cpp:11:6:11:15 | definition of myFunction | | 0 | | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | +| macro.c:2:1:2:3 | declaration of foo | | 2 | c_linkage, static | +| macro.c:4:5:4:8 | definition of main | | 1 | c_linkage | diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.ql b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.ql index 7d120121e88..fbc97861f4b 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.ql +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.ql @@ -2,4 +2,4 @@ import cpp from FunctionDeclarationEntry fde, string imp where if fde.isImplicit() then imp = "isImplicit" else imp = "" -select fde, imp +select fde, imp, count(fde.getASpecifier()), concat(fde.getASpecifier().toString(), ", ") diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/macro.c b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/macro.c new file mode 100644 index 00000000000..590b6a92b95 --- /dev/null +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/macro.c @@ -0,0 +1,7 @@ +#define Foo static void foo() +Foo; + +int main() +{ + return 0; +} diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected index 8f312e7f47f..07c0a9fdd97 100644 --- a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected +++ b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected @@ -1238,3 +1238,47 @@ ssa.cpp: # 254| v254_10(void) = UnmodeledUse : mu* # 254| v254_11(void) = AliasedUse : ~m262_1 # 254| v254_12(void) = ExitFunction : + +# 268| void* MallocAliasing(void*, int) +# 268| Block 0 +# 268| v268_1(void) = EnterFunction : +# 268| m268_2(unknown) = AliasedDefinition : +# 268| m268_3(unknown) = InitializeNonLocal : +# 268| m268_4(unknown) = Chi : total:m268_2, partial:m268_3 +# 268| mu268_5(unknown) = UnmodeledDefinition : +# 268| r268_6(glval) = VariableAddress[s] : +# 268| m268_7(void *) = InitializeParameter[s] : &:r268_6 +# 268| r268_8(void *) = Load : &:r268_6, m268_7 +# 268| m268_9(unknown) = InitializeIndirection[s] : &:r268_8 +# 268| r268_10(glval) = VariableAddress[size] : +# 268| m268_11(int) = InitializeParameter[size] : &:r268_10 +# 269| r269_1(glval) = VariableAddress[buf] : +# 269| r269_2(glval) = FunctionAddress[malloc] : +# 269| r269_3(glval) = VariableAddress[size] : +# 269| r269_4(int) = Load : &:r269_3, m268_11 +# 269| r269_5(void *) = Call : func:r269_2, 0:r269_4 +# 269| m269_6(unknown) = ^CallSideEffect : ~m268_9 +# 269| m269_7(unknown) = Chi : total:m268_9, partial:m269_6 +# 269| m269_8(unknown) = ^InitializeDynamicAllocation : &:r269_5 +# 269| m269_9(void *) = Store : &:r269_1, r269_5 +# 270| r270_1(glval) = FunctionAddress[memcpy] : +# 270| r270_2(glval) = VariableAddress[buf] : +# 270| r270_3(void *) = Load : &:r270_2, m269_9 +# 270| r270_4(glval) = VariableAddress[s] : +# 270| r270_5(void *) = Load : &:r270_4, m268_7 +# 270| r270_6(glval) = VariableAddress[size] : +# 270| r270_7(int) = Load : &:r270_6, m268_11 +# 270| r270_8(void *) = Call : func:r270_1, 0:r270_3, 1:r270_5, 2:r270_7 +# 270| v270_9(void) = ^SizedBufferReadSideEffect[1] : &:r270_5, r270_7, ~m269_8 +# 270| m270_10(unknown) = ^SizedBufferMustWriteSideEffect[0] : &:r270_3, r270_7 +# 270| m270_11(unknown) = Chi : total:m269_8, partial:m270_10 +# 271| r271_1(glval) = VariableAddress[#return] : +# 271| r271_2(glval) = VariableAddress[buf] : +# 271| r271_3(void *) = Load : &:r271_2, m269_9 +# 271| m271_4(void *) = Store : &:r271_1, r271_3 +# 268| v268_12(void) = ReturnIndirection : &:r268_8, ~m270_11 +# 268| r268_13(glval) = VariableAddress[#return] : +# 268| v268_14(void) = ReturnValue : &:r268_13, m271_4 +# 268| v268_15(void) = UnmodeledUse : mu* +# 268| v268_16(void) = AliasedUse : ~m270_11 +# 268| v268_17(void) = ExitFunction : diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected index c09c280bf9b..c6e1955c12d 100644 --- a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected +++ b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected @@ -1233,3 +1233,47 @@ ssa.cpp: # 254| v254_10(void) = UnmodeledUse : mu* # 254| v254_11(void) = AliasedUse : ~m262_1 # 254| v254_12(void) = ExitFunction : + +# 268| void* MallocAliasing(void*, int) +# 268| Block 0 +# 268| v268_1(void) = EnterFunction : +# 268| m268_2(unknown) = AliasedDefinition : +# 268| m268_3(unknown) = InitializeNonLocal : +# 268| m268_4(unknown) = Chi : total:m268_2, partial:m268_3 +# 268| mu268_5(unknown) = UnmodeledDefinition : +# 268| r268_6(glval) = VariableAddress[s] : +# 268| m268_7(void *) = InitializeParameter[s] : &:r268_6 +# 268| r268_8(void *) = Load : &:r268_6, m268_7 +# 268| m268_9(unknown) = InitializeIndirection[s] : &:r268_8 +# 268| r268_10(glval) = VariableAddress[size] : +# 268| m268_11(int) = InitializeParameter[size] : &:r268_10 +# 269| r269_1(glval) = VariableAddress[buf] : +# 269| r269_2(glval) = FunctionAddress[malloc] : +# 269| r269_3(glval) = VariableAddress[size] : +# 269| r269_4(int) = Load : &:r269_3, m268_11 +# 269| r269_5(void *) = Call : func:r269_2, 0:r269_4 +# 269| m269_6(unknown) = ^CallSideEffect : ~m268_4 +# 269| m269_7(unknown) = Chi : total:m268_4, partial:m269_6 +# 269| m269_8(unknown) = ^InitializeDynamicAllocation : &:r269_5 +# 269| m269_9(void *) = Store : &:r269_1, r269_5 +# 270| r270_1(glval) = FunctionAddress[memcpy] : +# 270| r270_2(glval) = VariableAddress[buf] : +# 270| r270_3(void *) = Load : &:r270_2, m269_9 +# 270| r270_4(glval) = VariableAddress[s] : +# 270| r270_5(void *) = Load : &:r270_4, m268_7 +# 270| r270_6(glval) = VariableAddress[size] : +# 270| r270_7(int) = Load : &:r270_6, m268_11 +# 270| r270_8(void *) = Call : func:r270_1, 0:r270_3, 1:r270_5, 2:r270_7 +# 270| v270_9(void) = ^SizedBufferReadSideEffect[1] : &:r270_5, r270_7, ~m268_9 +# 270| m270_10(unknown) = ^SizedBufferMustWriteSideEffect[0] : &:r270_3, r270_7 +# 270| m270_11(unknown) = Chi : total:m269_8, partial:m270_10 +# 271| r271_1(glval) = VariableAddress[#return] : +# 271| r271_2(glval) = VariableAddress[buf] : +# 271| r271_3(void *) = Load : &:r271_2, m269_9 +# 271| m271_4(void *) = Store : &:r271_1, r271_3 +# 268| v268_12(void) = ReturnIndirection : &:r268_8, m268_9 +# 268| r268_13(glval) = VariableAddress[#return] : +# 268| v268_14(void) = ReturnValue : &:r268_13, m271_4 +# 268| v268_15(void) = UnmodeledUse : mu* +# 268| v268_16(void) = AliasedUse : ~m269_7 +# 268| v268_17(void) = ExitFunction : diff --git a/cpp/ql/test/library-tests/ir/ssa/ssa.cpp b/cpp/ql/test/library-tests/ir/ssa/ssa.cpp index 5bdfd254e8f..ee75a2f28e4 100644 --- a/cpp/ql/test/library-tests/ir/ssa/ssa.cpp +++ b/cpp/ql/test/library-tests/ir/ssa/ssa.cpp @@ -262,3 +262,11 @@ char StringLiteralAliasing2(bool b) { const char* s = "Literal"; return s[2]; } + +void *malloc(int size); + +void *MallocAliasing(void *s, int size) { + void *buf = malloc(size); + memcpy(buf, s, size); + return buf; +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected index 213f97589d6..fb7a6eccc15 100644 --- a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected +++ b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected @@ -1147,3 +1147,44 @@ ssa.cpp: # 254| v254_9(void) = UnmodeledUse : mu* # 254| v254_10(void) = AliasedUse : ~mu254_4 # 254| v254_11(void) = ExitFunction : + +# 268| void* MallocAliasing(void*, int) +# 268| Block 0 +# 268| v268_1(void) = EnterFunction : +# 268| mu268_2(unknown) = AliasedDefinition : +# 268| mu268_3(unknown) = InitializeNonLocal : +# 268| mu268_4(unknown) = UnmodeledDefinition : +# 268| r268_5(glval) = VariableAddress[s] : +# 268| m268_6(void *) = InitializeParameter[s] : &:r268_5 +# 268| r268_7(void *) = Load : &:r268_5, m268_6 +# 268| mu268_8(unknown) = InitializeIndirection[s] : &:r268_7 +# 268| r268_9(glval) = VariableAddress[size] : +# 268| m268_10(int) = InitializeParameter[size] : &:r268_9 +# 269| r269_1(glval) = VariableAddress[buf] : +# 269| r269_2(glval) = FunctionAddress[malloc] : +# 269| r269_3(glval) = VariableAddress[size] : +# 269| r269_4(int) = Load : &:r269_3, m268_10 +# 269| r269_5(void *) = Call : func:r269_2, 0:r269_4 +# 269| mu269_6(unknown) = ^CallSideEffect : ~mu268_4 +# 269| mu269_7(unknown) = ^InitializeDynamicAllocation : &:r269_5 +# 269| m269_8(void *) = Store : &:r269_1, r269_5 +# 270| r270_1(glval) = FunctionAddress[memcpy] : +# 270| r270_2(glval) = VariableAddress[buf] : +# 270| r270_3(void *) = Load : &:r270_2, m269_8 +# 270| r270_4(glval) = VariableAddress[s] : +# 270| r270_5(void *) = Load : &:r270_4, m268_6 +# 270| r270_6(glval) = VariableAddress[size] : +# 270| r270_7(int) = Load : &:r270_6, m268_10 +# 270| r270_8(void *) = Call : func:r270_1, 0:r270_3, 1:r270_5, 2:r270_7 +# 270| v270_9(void) = ^SizedBufferReadSideEffect[1] : &:r270_5, r270_7, ~mu268_4 +# 270| mu270_10(unknown) = ^SizedBufferMustWriteSideEffect[0] : &:r270_3, r270_7 +# 271| r271_1(glval) = VariableAddress[#return] : +# 271| r271_2(glval) = VariableAddress[buf] : +# 271| r271_3(void *) = Load : &:r271_2, m269_8 +# 271| m271_4(void *) = Store : &:r271_1, r271_3 +# 268| v268_11(void) = ReturnIndirection : &:r268_7, ~mu268_4 +# 268| r268_12(glval) = VariableAddress[#return] : +# 268| v268_13(void) = ReturnValue : &:r268_12, m271_4 +# 268| v268_14(void) = UnmodeledUse : mu* +# 268| v268_15(void) = AliasedUse : ~mu268_4 +# 268| v268_16(void) = ExitFunction : diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir_unsound.expected b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir_unsound.expected index 213f97589d6..fb7a6eccc15 100644 --- a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir_unsound.expected +++ b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir_unsound.expected @@ -1147,3 +1147,44 @@ ssa.cpp: # 254| v254_9(void) = UnmodeledUse : mu* # 254| v254_10(void) = AliasedUse : ~mu254_4 # 254| v254_11(void) = ExitFunction : + +# 268| void* MallocAliasing(void*, int) +# 268| Block 0 +# 268| v268_1(void) = EnterFunction : +# 268| mu268_2(unknown) = AliasedDefinition : +# 268| mu268_3(unknown) = InitializeNonLocal : +# 268| mu268_4(unknown) = UnmodeledDefinition : +# 268| r268_5(glval) = VariableAddress[s] : +# 268| m268_6(void *) = InitializeParameter[s] : &:r268_5 +# 268| r268_7(void *) = Load : &:r268_5, m268_6 +# 268| mu268_8(unknown) = InitializeIndirection[s] : &:r268_7 +# 268| r268_9(glval) = VariableAddress[size] : +# 268| m268_10(int) = InitializeParameter[size] : &:r268_9 +# 269| r269_1(glval) = VariableAddress[buf] : +# 269| r269_2(glval) = FunctionAddress[malloc] : +# 269| r269_3(glval) = VariableAddress[size] : +# 269| r269_4(int) = Load : &:r269_3, m268_10 +# 269| r269_5(void *) = Call : func:r269_2, 0:r269_4 +# 269| mu269_6(unknown) = ^CallSideEffect : ~mu268_4 +# 269| mu269_7(unknown) = ^InitializeDynamicAllocation : &:r269_5 +# 269| m269_8(void *) = Store : &:r269_1, r269_5 +# 270| r270_1(glval) = FunctionAddress[memcpy] : +# 270| r270_2(glval) = VariableAddress[buf] : +# 270| r270_3(void *) = Load : &:r270_2, m269_8 +# 270| r270_4(glval) = VariableAddress[s] : +# 270| r270_5(void *) = Load : &:r270_4, m268_6 +# 270| r270_6(glval) = VariableAddress[size] : +# 270| r270_7(int) = Load : &:r270_6, m268_10 +# 270| r270_8(void *) = Call : func:r270_1, 0:r270_3, 1:r270_5, 2:r270_7 +# 270| v270_9(void) = ^SizedBufferReadSideEffect[1] : &:r270_5, r270_7, ~mu268_4 +# 270| mu270_10(unknown) = ^SizedBufferMustWriteSideEffect[0] : &:r270_3, r270_7 +# 271| r271_1(glval) = VariableAddress[#return] : +# 271| r271_2(glval) = VariableAddress[buf] : +# 271| r271_3(void *) = Load : &:r271_2, m269_8 +# 271| m271_4(void *) = Store : &:r271_1, r271_3 +# 268| v268_11(void) = ReturnIndirection : &:r268_7, ~mu268_4 +# 268| r268_12(glval) = VariableAddress[#return] : +# 268| v268_13(void) = ReturnValue : &:r268_12, m271_4 +# 268| v268_14(void) = UnmodeledUse : mu* +# 268| v268_15(void) = AliasedUse : ~mu268_4 +# 268| v268_16(void) = ExitFunction : diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll index 886183c32ba..83c9ac1215c 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll @@ -82,6 +82,7 @@ private newtype TOpcode = TSizedBufferReadSideEffect() or TSizedBufferMustWriteSideEffect() or TSizedBufferMayWriteSideEffect() or + TInitializeDynamicAllocation() or TChi() or TInlineAsm() or TUnreached() or @@ -213,23 +214,28 @@ abstract class IndirectReadOpcode extends IndirectMemoryAccessOpcode { } /** - * An opcode that accesses a memory buffer of unknown size. + * An opcode that accesses a memory buffer. */ abstract class BufferAccessOpcode extends Opcode { final override predicate hasAddressOperand() { any() } } +/** + * An opcode that accesses a memory buffer of unknown size. + */ +abstract class UnsizedBufferAccessOpcode extends BufferAccessOpcode { } + /** * An opcode that writes to a memory buffer of unknown size. */ -abstract class BufferWriteOpcode extends BufferAccessOpcode { +abstract class UnsizedBufferWriteOpcode extends UnsizedBufferAccessOpcode { final override MemoryAccessKind getWriteMemoryAccess() { result instanceof BufferMemoryAccess } } /** * An opcode that reads from a memory buffer of unknown size. */ -abstract class BufferReadOpcode extends BufferAccessOpcode { +abstract class UnsizedBufferReadOpcode extends UnsizedBufferAccessOpcode { final override MemoryAccessKind getReadMemoryAccess() { result instanceof BufferMemoryAccess } } @@ -261,9 +267,7 @@ abstract class EntireAllocationReadOpcode extends EntireAllocationAccessOpcode { /** * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`. */ -abstract class SizedBufferAccessOpcode extends Opcode { - final override predicate hasAddressOperand() { any() } - +abstract class SizedBufferAccessOpcode extends BufferAccessOpcode { final override predicate hasBufferSizeOperand() { any() } } @@ -666,17 +670,18 @@ module Opcode { final override string toString() { result = "IndirectMayWriteSideEffect" } } - class BufferReadSideEffect extends ReadSideEffectOpcode, BufferReadOpcode, TBufferReadSideEffect { + class BufferReadSideEffect extends ReadSideEffectOpcode, UnsizedBufferReadOpcode, + TBufferReadSideEffect { final override string toString() { result = "BufferReadSideEffect" } } - class BufferMustWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, + class BufferMustWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, TBufferMustWriteSideEffect { final override string toString() { result = "BufferMustWriteSideEffect" } } - class BufferMayWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, MayWriteOpcode, - TBufferMayWriteSideEffect { + class BufferMayWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, + MayWriteOpcode, TBufferMayWriteSideEffect { final override string toString() { result = "BufferMayWriteSideEffect" } } @@ -695,6 +700,11 @@ module Opcode { final override string toString() { result = "SizedBufferMayWriteSideEffect" } } + class InitializeDynamicAllocation extends SideEffectOpcode, EntireAllocationWriteOpcode, + TInitializeDynamicAllocation { + final override string toString() { result = "InitializeDynamicAllocation" } + } + class Chi extends Opcode, TChi { final override string toString() { result = "Chi" } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll index ee7aebc4749..ae585ec2c7e 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll @@ -176,7 +176,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { /** * A temporary variable introduced by IR construction. The most common examples are the variable - * generated to hold the return value of afunction, or the variable generated to hold the result of + * generated to hold the return value of a function, or the variable generated to hold the result of * a condition operator (`a ? b : c`). */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { 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 d058a6c5a3e..d14cdf08910 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 @@ -1350,6 +1350,26 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } +/** + * An instruction representing the initial value of newly allocated memory, e.g. the result of a + * call to `malloc`. + */ +class InitializeDynamicAllocationInstruction extends SideEffectInstruction { + InitializeDynamicAllocationInstruction() { + getOpcode() instanceof Opcode::InitializeDynamicAllocation + } + + /** + * Gets the address of the allocation this instruction is initializing. + */ + final AddressOperand getAllocationAddressOperand() { result = getAnOperand() } + + /** + * Gets the operand for the allocation this instruction is initializing. + */ + final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() } +} + /** * An instruction representing a GNU or MSVC inline assembly statement. */ diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll index ee7aebc4749..ae585ec2c7e 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll @@ -176,7 +176,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { /** * A temporary variable introduced by IR construction. The most common examples are the variable - * generated to hold the return value of afunction, or the variable generated to hold the result of + * generated to hold the return value of a function, or the variable generated to hold the result of * a condition operator (`a ? b : c`). */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { 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 d058a6c5a3e..d14cdf08910 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 @@ -1350,6 +1350,26 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } +/** + * An instruction representing the initial value of newly allocated memory, e.g. the result of a + * call to `malloc`. + */ +class InitializeDynamicAllocationInstruction extends SideEffectInstruction { + InitializeDynamicAllocationInstruction() { + getOpcode() instanceof Opcode::InitializeDynamicAllocation + } + + /** + * Gets the address of the allocation this instruction is initializing. + */ + final AddressOperand getAllocationAddressOperand() { result = getAnOperand() } + + /** + * Gets the operand for the allocation this instruction is initializing. + */ + final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() } +} + /** * An instruction representing a GNU or MSVC inline assembly statement. */ diff --git a/java/ql/src/Customizations.qll b/java/ql/src/Customizations.qll new file mode 100644 index 00000000000..1f5716726e3 --- /dev/null +++ b/java/ql/src/Customizations.qll @@ -0,0 +1,12 @@ +/** + * Contains customizations to the standard library. + * + * This module is imported by `java.qll`, so any customizations defined here automatically + * apply to all queries. + * + * Typical examples of customizations include adding new subclasses of abstract classes such as + * the `RemoteFlowSource` and `AdditionalTaintStep` classes associated with the security queries + * to model frameworks that are not covered by the standard library. + */ + +import java diff --git a/java/ql/src/java.qll b/java/ql/src/java.qll index c8e3b71f18b..6454a0d282b 100644 --- a/java/ql/src/java.qll +++ b/java/ql/src/java.qll @@ -1,5 +1,6 @@ /** Provides all default Java QL imports. */ +import Customizations import semmle.code.FileSystem import semmle.code.Location import semmle.code.java.Annotation diff --git a/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql b/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql index 5d7234ef504..2a00a49c020 100644 --- a/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql +++ b/javascript/ql/examples/queries/dataflow/StoredXss/StoredXss.ql @@ -22,7 +22,8 @@ import DataFlow::PathGraph */ class MysqlSource extends StoredXss::Source { MysqlSource() { - this = moduleImport("mysql") + this = + moduleImport("mysql") .getAMemberCall("createConnection") .getAMethodCall("query") .getCallback(1) diff --git a/javascript/ql/src/Security/CWE-078/UselessUseOfCat.qhelp b/javascript/ql/src/Security/CWE-078/UselessUseOfCat.qhelp new file mode 100644 index 00000000000..5ef218bdf59 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/UselessUseOfCat.qhelp @@ -0,0 +1,45 @@ + + + +

Using the unix command cat only to read a file is an +unnecessarily complex way to achieve something that can be done in a simpler +and safer manner using the Node.js fs.readFile API. +

+

+The use of cat for simple file reads leads to code that is +unportable, inefficient, complex, and can lead to subtle bugs or even +security vulnerabilities. +

+
+ +

+Use fs.readFile or fs.readFileSync to read files +from the file system. +

+
+ + +

The following example shows code that reads a file using cat:

+ + + +

The code in the example will break if the input name contains +special characters (including space). Additionally, it does not work on Windows +and if the input is user-controlled, a command injection attack can happen.

+ +

The fs.readFile API should be used to avoid these potential issues:

+ + + +
+ + +
  • OWASP: Command Injection.
  • +
  • Node.js: File System API.
  • +
  • The Useless Use of Cat Award.
  • + + +
    +
    diff --git a/javascript/ql/src/Security/CWE-078/UselessUseOfCat.ql b/javascript/ql/src/Security/CWE-078/UselessUseOfCat.ql new file mode 100644 index 00000000000..6b0ed59e632 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/UselessUseOfCat.ql @@ -0,0 +1,25 @@ +/** + * @name Unnecessary use of `cat` process + * @description Using the `cat` process to read a file is unnecessarily complex, inefficient, unportable, and can lead to subtle bugs, or even security vulnerabilities. + * @kind problem + * @problem.severity error + * @precision high + * @id js/unnecessary-use-of-cat + * @tags correctness + * security + * maintainability + */ + +import javascript +import semmle.javascript.security.UselessUseOfCat +import semmle.javascript.RestrictedLocations + +from UselessCat cat, string message +where + message = " Can be replaced with: " + PrettyPrintCatCall::createReadFileCall(cat) + or + not exists(PrettyPrintCatCall::createReadFileCall(cat)) and + if cat.isSync() + then message = " Can be replaced with a call to fs.readFileSync(..)." + else message = " Can be replaced with a call to fs.readFile(..)." +select cat.asExpr().(FirstLineOf), "Unnecessary use of `cat` process." + message diff --git a/javascript/ql/src/Security/CWE-078/examples/useless-cat-fixed.js b/javascript/ql/src/Security/CWE-078/examples/useless-cat-fixed.js new file mode 100644 index 00000000000..225fa1f5869 --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/examples/useless-cat-fixed.js @@ -0,0 +1,5 @@ +var fs = require('fs'); + +module.exports = function (name) { + return fs.readFileSync(name).toString(); +}; diff --git a/javascript/ql/src/Security/CWE-078/examples/useless-cat.js b/javascript/ql/src/Security/CWE-078/examples/useless-cat.js new file mode 100644 index 00000000000..78f099d0e4c --- /dev/null +++ b/javascript/ql/src/Security/CWE-078/examples/useless-cat.js @@ -0,0 +1,5 @@ +var child_process = require('child_process'); + +module.exports = function (name) { + return child_process.execSync("cat " + name).toString(); +}; diff --git a/javascript/ql/src/semmle/javascript/Concepts.qll b/javascript/ql/src/semmle/javascript/Concepts.qll index 23bbbce3b16..798747af8b8 100644 --- a/javascript/ql/src/semmle/javascript/Concepts.qll +++ b/javascript/ql/src/semmle/javascript/Concepts.qll @@ -22,6 +22,14 @@ abstract class SystemCommandExecution extends DataFlow::Node { * to the command. */ DataFlow::Node getArgumentList() { none() } + + /** Holds if the command execution happens synchronously. */ + abstract predicate isSync(); + + /** + * Gets the data-flow node (if it exists) for an options argument. + */ + abstract DataFlow::Node getOptionsArg(); } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll index 1fca6e624ed..cee84c9b460 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll @@ -632,6 +632,22 @@ module NodeJSLib { // all of the above methods take the argument list as their second argument result = getArgument(1) } + + override predicate isSync() { + "Sync" = methodName.suffix(methodName.length() - 4) + } + + override DataFlow::Node getOptionsArg() { + not result.getALocalSource() instanceof DataFlow::FunctionNode and // looks like callback + not result.getALocalSource() instanceof DataFlow::ArrayCreationNode and // looks like argumentlist + not result = getArgument(0) and + // fork/spawn and all sync methos always has options as the last argument + if methodName.regexpMatch("fork.*") or methodName.regexpMatch("spawn.*") or methodName.regexpMatch(".*Sync") then + result = getLastArgument() + else + // the rest (exec/execFile) has the options argument as their second last. + result = getArgument(this.getNumArgument() - 2) + } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll b/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll index 22605479244..6944b7e74a3 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/ShellJS.qll @@ -160,6 +160,15 @@ module ShellJS { override DataFlow::Node getACommandArgument() { result = getArgument(0) } override predicate isShellInterpreted(DataFlow::Node arg) { arg = getACommandArgument() } + + override predicate isSync() {none ()} + + override DataFlow::Node getOptionsArg() { + result = getLastArgument() and + not result = getArgument(0) and + not result.getALocalSource() instanceof DataFlow::FunctionNode and // looks like callback + not result.getALocalSource() instanceof DataFlow::ArrayCreationNode // looks like argumentlist + } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll b/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll index 32bfd8f70f7..8c9535d5f7e 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/SystemCommandExecutors.qll @@ -7,14 +7,17 @@ import javascript private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::InvokeNode { int cmdArg; + int optionsArg; // either a positive number representing the n'th argument, or a negative number representing the n'th last argument (e.g. -2 is the second last argument). boolean shell; + boolean sync; SystemCommandExecutors() { exists(string mod, DataFlow::SourceNode callee | exists(string method | - mod = "cross-spawn" and method = "sync" and cmdArg = 0 and shell = false + mod = "cross-spawn" and method = "sync" and cmdArg = 0 and shell = false and optionsArg = -1 or mod = "execa" and + optionsArg = -1 and ( shell = false and ( @@ -30,27 +33,30 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I ) and cmdArg = 0 | - callee = DataFlow::moduleMember(mod, method) + callee = DataFlow::moduleMember(mod, method) and + sync = getSync(method) ) or + sync = false and ( shell = false and ( - mod = "cross-spawn" and cmdArg = 0 + mod = "cross-spawn" and cmdArg = 0 and optionsArg = -1 or - mod = "cross-spawn-async" and cmdArg = 0 + mod = "cross-spawn-async" and cmdArg = 0 and optionsArg = -1 or - mod = "exec-async" and cmdArg = 0 + mod = "exec-async" and cmdArg = 0 and optionsArg = -1 or - mod = "execa" and cmdArg = 0 + mod = "execa" and cmdArg = 0 and optionsArg = -1 ) or shell = true and ( mod = "exec" and + optionsArg = -2 and cmdArg = 0 or - mod = "remote-exec" and cmdArg = 1 + mod = "remote-exec" and cmdArg = 1 and optionsArg = -1 ) ) and callee = DataFlow::moduleImport(mod) @@ -64,4 +70,30 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I override predicate isShellInterpreted(DataFlow::Node arg) { arg = getACommandArgument() and shell = true } + + override DataFlow::Node getArgumentList() { shell = false and result = getArgument(1) } + + override predicate isSync() { sync = true } + + override DataFlow::Node getOptionsArg() { + ( + if optionsArg < 0 + then + result = getArgument(getNumArgument() + optionsArg) and + getNumArgument() + optionsArg > cmdArg + else result = getArgument(optionsArg) + ) and + not result.getALocalSource() instanceof DataFlow::FunctionNode and // looks like callback + not result.getALocalSource() instanceof DataFlow::ArrayCreationNode // looks like argumentlist + } +} + +/** + * Gets a boolean reflecting if the name ends with "sync" or "Sync". + */ +bindingset[name] +private boolean getSync(string name) { + if name.suffix(name.length() - 4) = "Sync" or name.suffix(name.length() - 4) = "sync" + then result = true + else result = false } diff --git a/javascript/ql/src/semmle/javascript/security/UselessUseOfCat.qll b/javascript/ql/src/semmle/javascript/security/UselessUseOfCat.qll new file mode 100644 index 00000000000..4c8b3815586 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/UselessUseOfCat.qll @@ -0,0 +1,331 @@ +/** + * Provides predicates and classes for working with useless uses of the unix command `cat`. + */ + +import javascript +import Expressions.ExprHasNoEffect +import Declarations.UnusedVariable + +/** + * A call that executes a system command. + * This class provides utility predicates for reasoning about command execution calls. + */ +private class CommandCall extends DataFlow::InvokeNode { + SystemCommandExecution command; + + CommandCall() { this = command } + + /** + * Holds if the call is synchronous (e.g. `execFileSync`). + */ + predicate isSync() { command.isSync() } + + /** + * Gets a list that specifies the arguments given to the command. + */ + DataFlow::ArrayCreationNode getArgumentList() { result = command.getArgumentList().getALocalSource() } + + /** + * Gets the callback (if it exists) for an async `exec`-like call. + */ + DataFlow::FunctionNode getCallback() { + not this.isSync() and result = getLastArgument().getALocalSource() + } + + /** + * Holds if the executed command execution has an argument list as a separate argument. + */ + predicate hasArgumentList() { exists(getArgumentList()) } + + /** + * Gets the data-flow node (if it exists) for an options argument for an `exec`-like call. + */ + DataFlow::Node getOptionsArg() { result = command.getOptionsArg() } + + /** + * Gets the constant-string parts that are not part of the command itself. + * E.g. for a command execution `exec("/bin/cat foo bar")` this predicate will have result `"foo bar"`. + */ + string getNonCommandConstantString() { + if this.hasArgumentList() + then + result = + getConstantStringParts(getArgumentList() + .getALocalSource() + .(DataFlow::ArrayCreationNode) + .getElement(_)) + else + exists(string commandString | commandString = getConstantStringParts(getArgument(0)) | + result = commandString.suffix(1 + commandString.indexOf(" ", 0, 0)) + ) + } + + /** + * Holds if this command execution invokes the executeable `name`. + */ + bindingset[name] + predicate isACallTo(string name) { + if this.hasArgumentList() + then getArgument(0).mayHaveStringValue(name) + else + exists(string arg | arg = getConstantStringParts(getArgument(0)) | + arg.prefix(name.length()) = name + ) + } +} + +/** + * Holds if the input `str` contains some character that might be interpreted in a non-trivial way by a shell. + */ +bindingset[str] +private predicate containsNonTrivialShellChar(string str) { + exists(str.regexpFind("\\*|\\||>|<| |\\$|&|,|\\`| |;", _, _)) +} + +/** + * Gets the constant string parts from a data-flow node. + * Either the result is a constant string value that the node can hold, or the node is a string-concatenation and the result is the string parts from the concatenation. + */ +private string getConstantStringParts(DataFlow::Node node) { + node.mayHaveStringValue(result) + or + result = node.(StringOps::ConcatenationRoot).getConstantStringParts() +} + +/** + * A call to a useless use of `cat`. + */ +class UselessCat extends CommandCall { + UselessCat() { + this = command and + isACallTo(getACatExecuteable()) and + // There is a file to read, it's not just spawning `cat`. + not ( + not exists(getArgumentList()) and + getArgument(0).mayHaveStringValue(getACatExecuteable()) + ) and + // wildcards, pipes, redirections, other bash features, and multiple files (spaces) are OK. + not containsNonTrivialShellChar(getNonCommandConstantString()) and + // Only acceptable option is "encoding", everything else is non-trivial to emulate with fs.readFile. + ( + not exists(getOptionsArg()) + or + forex(string prop | exists(getOptionsArg().getALocalSource().getAPropertyWrite(prop)) | + prop = "encoding" + ) + ) and + // If there is a callback, then it must either have one or two parameters, or if there is a third parameter it must be unused. + ( + not exists(getCallback()) + or + exists(DataFlow::FunctionNode func | func = getCallback() | + func.getNumParameter() = 1 + or + func.getNumParameter() = 2 + or + // `exec` can use 3 parameters, `readFile` can only use two, so it is OK to have a third parameter if it is unused, + func.getNumParameter() = 3 and + not exists(SSA::definition(func.getParameter(2).getParameter())) + ) + ) and + // The process returned by an async call is unused. + ( + isSync() + or + inVoidContext(this.getEnclosingExpr()) + or + this.getEnclosingExpr() = any(UnusedLocal v).getAnAssignedExpr() + ) + } +} + +/** + * Gets a string used to call `cat`. + */ +private string getACatExecuteable() { + result = "cat" or result = "/bin/cat" +} + +/** + * Predicates for creating an equivalent call to `fs.readFile` from a command execution of `cat`. + */ +module PrettyPrintCatCall { + /** + * Create a string representation of an equivalent call to `fs.readFile` for a given command execution `cat`. + */ + string createReadFileCall(UselessCat cat) { + exists(string sync, string extraArg, string callback, string fileArg | + (if cat.isSync() then sync = "Sync" else sync = "") and + ( + exists(cat.getOptionsArg()) and + ( + extraArg = ", " + createOptionsArg(cat.getOptionsArg()) + or + not exists(createOptionsArg(cat.getOptionsArg())) and + extraArg = ", ..." + ) + or + extraArg = "" and not exists(cat.getOptionsArg()) + ) and + ( + callback = createCallbackString(cat.getCallback()) + or + callback = "" and not exists(cat.getCallback()) + ) and + fileArg = createFileArgument(cat).trim() and + // sanity check in case of surprising `toString` results, other uses of `containsNonTrivialBashChar` should ensure that this conjunct will hold most of the time + not(containsNonTrivialShellChar(fileArg.regexpReplaceAll("\\$|\\`| ", ""))) // string concat might contain " ", template strings might contain "$" or `, and that is OK. + | + result = + "fs.readFile" + sync + "(" + fileArg + extraArg + callback + ")" + ) + } + + /** + * Create a string representation of the expression that determines what file is read by `cat`. + */ + string createFileArgument(CommandCall cat) { + if cat.hasArgumentList() + then + cat.getArgument(0).mayHaveStringValue(getACatExecuteable()) and + result = createFileThatIsReadFromCommandList(cat) + else result = createFileArgumentWithoutCat(cat.getArgument(0)) + } + + /** + * Create a string representing the callback `func`. + */ + string createCallbackString(DataFlow::FunctionNode func) { + exists(string params | params = createCallbackParams(func) | + if func.getFunction() instanceof ArrowFunctionExpr + then + if func.getFunction().getBody() instanceof Expr + then result = ", (" + params + ") => ..." + else result = ", (" + params + ") => {...}" + else result = ", function(" + params + ") {...}" + ) + } + + /** + * Create a string concatenation of the parameter names in a function `func`. + */ + private string createCallbackParams(DataFlow::FunctionNode func) { + result = + concat(int i | + i = [0 .. func.getNumParameter()] + | + func.getParameter(i).getName(), ", " order by i + ) + } + + /** + * Create a string representation of the options argument `arg` from an exec-like call. + */ + private string createOptionsArg(DataFlow::Node arg) { + result = arg.asExpr().(VarAccess).getVariable().getName() + or + // fall back to toString(), but ensure that we don't have dots in the middle. + result = arg.(DataFlow::ObjectLiteralNode).toString() and not result.regexpMatch(".*\\.\\..*") + } + + /** + * Create a string representation of a string concatenation. + */ + private string createConcatRepresentation(StringOps::ConcatenationRoot root) { + // String concat + not exists(root.getStringValue()) and + not root.asExpr() instanceof TemplateLiteral and + forall(Expr e | e = root.getALeaf().asExpr() | exists(createLeafRepresentation(e))) and + result = + concat(Expr leaf | + leaf = root.getALeaf().asExpr() + | + createLeafRepresentation(leaf), " + " order by leaf.getFirstToken().getIndex() + ) + or + // Template string + exists(TemplateLiteral template | template = root.asExpr() | + forall(Expr e | e = template.getAChild() | exists(createTemplateElementRepresentation(e))) and + result = + "`" + + concat(int i | + i = [0 .. template.getNumChild() - 1] + | + createTemplateElementRepresentation(template.getChild(i)) order by i + ) + "`" + ) + } + + /** + * Create a string representing the expression needed to re-create the value for a leaf in a string-concatenation. + */ + private string createLeafRepresentation(Expr e) { + result = "\"" + e.getStringValue() + "\"" or + result = e.(VarAccess).getVariable().getName() + } + + /** + * Create a string representing the expression needed to re-create the value for an element of a template string. + */ + private string createTemplateElementRepresentation(Expr e) { + result = "${" + e.(VarAccess).getVariable().getName() + "}" + or + result = e.(TemplateElement).getValue() + } + + /** + * Create a string representing an expression that gets the file read by a call to `cat`. + * The input `arg` is the node that determines the commandline where `cat` is invoked. + */ + private string createFileArgumentWithoutCat(DataFlow::Node arg) { + exists(string cat | cat = getACatExecuteable() | + exists(string command | arg.mayHaveStringValue(command) | + command.prefix(cat.length()) = cat and + result = "\"" + command.suffix(cat.length()).trim() + "\"" + ) + or + exists(StringOps::ConcatenationRoot root, string printed, string quote | + root = arg and printed = createConcatRepresentation(root).suffix(1) // remove initial quote + | + (if root.asExpr() instanceof TemplateLiteral then quote = "`" else quote = "\"") and + root.getFirstLeaf().getStringValue().prefix(cat.length()) = cat and + exists(string rawConcat | rawConcat = quote + printed.suffix(cat.length()).trim() | + result = createSimplifiedStringConcat(rawConcat) + ) + ) + ) + } + + /** + * Create a simplified and equivalent string concatenation for a given string concatenation `str` + */ + bindingset[str] + private string createSimplifiedStringConcat(string str) { + // Remove an initial ""+ (e.g. in `""+file`) + if str.prefix(5) = "\"\" + " + then result = str.suffix(5) + else + // prettify `${newpath}` to just newpath + if + str.prefix(3) = "`${" and + str.suffix(str.length() - 2) = "}`" and + not str.suffix(3).matches("%{%") + then result = str.prefix(str.length() - 2).suffix(3) + else result = str + } + + /** + * Create the file that is read for a call with an explicit command list (e.g. `child_process.execFile/execFileSync`). + */ + string createFileThatIsReadFromCommandList(CommandCall call) { + exists(DataFlow::ArrayCreationNode array, DataFlow::Node element | + array = call.getArgumentList().(DataFlow::ArrayCreationNode) and + array.getSize() = 1 and + element = array.getElement(0) + | + result = element.asExpr().(VarAccess).getVariable().getName() or + result = "\"" + element.getStringValue() + "\"" or + result = createConcatRepresentation(element) + ) + } +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll index 8dca676af46..242504a9c59 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll @@ -36,7 +36,7 @@ module TaintedPath { guard instanceof StartsWithDirSanitizer or guard instanceof IsAbsoluteSanitizer or guard instanceof ContainsDotDotSanitizer or - guard instanceof RelativePathStartsWithDotDotSanitizer or + guard instanceof RelativePathStartsWithSanitizer or guard instanceof IsInsideCheckSanitizer } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll index d1be274314c..2ed29813835 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll @@ -368,29 +368,47 @@ module TaintedPath { * // pathname is safe * } * ``` + * + * or + * ``` + * var relative = path.resolve(pathname); // or path.normalize + * if(relative.startsWith(webroot) { + * // pathname is safe + * } else { + * // pathname is unsafe + * } + * ``` */ - class RelativePathStartsWithDotDotSanitizer extends DataFlow::BarrierGuardNode { + class RelativePathStartsWithSanitizer extends DataFlow::BarrierGuardNode { StringOps::StartsWith startsWith; - DataFlow::CallNode relativeCall; + DataFlow::CallNode pathCall; + string member; - RelativePathStartsWithDotDotSanitizer() { + RelativePathStartsWithSanitizer() { + (member = "relative" or member = "resolve" or member = "normalize") and this = startsWith and - relativeCall = NodeJSLib::Path::moduleMember("relative").getACall() and + pathCall = NodeJSLib::Path::moduleMember(member).getACall() and ( - startsWith.getBaseString().getALocalSource() = relativeCall + startsWith.getBaseString().getALocalSource() = pathCall or startsWith .getBaseString() .getALocalSource() .(NormalizingPathCall) .getInput() - .getALocalSource() = relativeCall + .getALocalSource() = pathCall ) and - isDotDotSlashPrefix(startsWith.getSubstring()) + (not member = "relative" or isDotDotSlashPrefix(startsWith.getSubstring())) } override predicate blocks(boolean outcome, Expr e) { - e = relativeCall.getArgument(1).asExpr() and outcome = startsWith.getPolarity().booleanNot() + member = "relative" and + e = pathCall.getArgument(1).asExpr() and + outcome = startsWith.getPolarity().booleanNot() + or + not member = "relative" and + e = pathCall.getArgument(0).asExpr() and + outcome = startsWith.getPolarity() } } diff --git a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.ql b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.ql index dd41c794213..06b9671c2f5 100644 --- a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.ql +++ b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.ql @@ -34,9 +34,7 @@ class AnnotatedCall extends InvokeExpr { AnnotatedFunction getAnExpectedCallee() { result.getCalleeName() = getCallTargetName() } - int getBoundArgs() { - result = getAnnotation(this, "boundArgs").toInt() - } + int getBoundArgs() { result = getAnnotation(this, "boundArgs").toInt() } int getBoundArgsOrMinusOne() { result = getBoundArgs() diff --git a/javascript/ql/test/library-tests/Classes/tests.ql b/javascript/ql/test/library-tests/Classes/tests.ql index 41fb13f3af0..5cb3c90db85 100644 --- a/javascript/ql/test/library-tests/Classes/tests.ql +++ b/javascript/ql/test/library-tests/Classes/tests.ql @@ -17,4 +17,4 @@ import ConstructorDefinitions import ClassNodeConstructor import ClassNodeInstanceMethod import PrivateField -import ClassFlow \ No newline at end of file +import ClassFlow diff --git a/javascript/ql/test/library-tests/CustomLoadStoreSteps/test.ql b/javascript/ql/test/library-tests/CustomLoadStoreSteps/test.ql index e4ffc89294d..0bde5a737a6 100644 --- a/javascript/ql/test/library-tests/CustomLoadStoreSteps/test.ql +++ b/javascript/ql/test/library-tests/CustomLoadStoreSteps/test.ql @@ -13,7 +13,7 @@ class Configuration extends TaintTracking::Configuration { // When the source code states that "foo" is being read, "bar" is additionally being read. override predicate isAdditionalLoadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { - exists(DataFlow::PropRead read | read = succ | + exists(DataFlow::PropRead read | read = succ | read.getBase() = pred and read.getPropertyName() = "foo" ) and diff --git a/javascript/ql/test/library-tests/DependencyModuleImports/Test.ql b/javascript/ql/test/library-tests/DependencyModuleImports/Test.ql index 9b8e63bf6d2..0552ff7b48a 100644 --- a/javascript/ql/test/library-tests/DependencyModuleImports/Test.ql +++ b/javascript/ql/test/library-tests/DependencyModuleImports/Test.ql @@ -32,9 +32,8 @@ class SampleVersionSink extends DataFlow::Node { functionName = "m1" and argNumber = 0 | - this = DataFlow::dependencyModuleImport(dep) - .getAMemberCall(functionName) - .getArgument(argNumber) and + this = + DataFlow::dependencyModuleImport(dep).getAMemberCall(functionName).getArgument(argNumber) and dep.info(dependencyName, vDep) and vDep.maybeBetween(vFrom, vTo) ) diff --git a/javascript/ql/test/library-tests/PartialInvokeNode/test.ql b/javascript/ql/test/library-tests/PartialInvokeNode/test.ql index a7e5a010a05..1c2093d0dad 100644 --- a/javascript/ql/test/library-tests/PartialInvokeNode/test.ql +++ b/javascript/ql/test/library-tests/PartialInvokeNode/test.ql @@ -1,21 +1,21 @@ import javascript -query -DataFlow::Node getBoundFunction(DataFlow::PartialInvokeNode invoke, DataFlow::Node callback, int boundArgs) { +query DataFlow::Node getBoundFunction( + DataFlow::PartialInvokeNode invoke, DataFlow::Node callback, int boundArgs +) { result = invoke.getBoundFunction(callback, boundArgs) } -query -predicate isPartialArgument(DataFlow::PartialInvokeNode invoke, DataFlow::Node callback, DataFlow::Node argument, int index) { +query predicate isPartialArgument( + DataFlow::PartialInvokeNode invoke, DataFlow::Node callback, DataFlow::Node argument, int index +) { invoke.isPartialArgument(callback, argument, index) } -query -DataFlow::Node getBoundReceiver(DataFlow::PartialInvokeNode invoke) { +query DataFlow::Node getBoundReceiver(DataFlow::PartialInvokeNode invoke) { result = invoke.getBoundReceiver() } -query -DataFlow::Node clickEvent() { +query DataFlow::Node clickEvent() { result = DataFlow::globalVarRef("addEventListener").getACall().getABoundCallbackParameter(1, 0) } diff --git a/javascript/ql/test/library-tests/Promises/tests.ql b/javascript/ql/test/library-tests/Promises/tests.ql index 2490f378970..bcb41981fe0 100644 --- a/javascript/ql/test/library-tests/Promises/tests.ql +++ b/javascript/ql/test/library-tests/Promises/tests.ql @@ -7,4 +7,4 @@ import PromiseDefinition_getAResolveHandler import PromiseDefinition_getRejectParameter import PromiseDefinition_getResolveParameter import PromiseDefinition_getACatchHandler -import flow \ No newline at end of file +import flow diff --git a/javascript/ql/test/library-tests/RangeAnalysis/DeadBranch.ql b/javascript/ql/test/library-tests/RangeAnalysis/DeadBranch.ql index 8e6a798d737..d1554c34933 100644 --- a/javascript/ql/test/library-tests/RangeAnalysis/DeadBranch.ql +++ b/javascript/ql/test/library-tests/RangeAnalysis/DeadBranch.ql @@ -22,8 +22,8 @@ class AssertionComment extends LineComment { isOK = true and exists(ConditionGuardNode guard | guard = getAGuardNode() | RangeAnalysis::isContradictoryGuardNode(guard) and - result = "Error: analysis claims " + getTestExpr() + " is always " + - guard.getOutcome().booleanNot() + result = + "Error: analysis claims " + getTestExpr() + " is always " + guard.getOutcome().booleanNot() ) or isOK = false and diff --git a/javascript/ql/test/library-tests/RegExp/EscapeInString/EscapeInString.ql b/javascript/ql/test/library-tests/RegExp/EscapeInString/EscapeInString.ql index f824fe01920..22fcecfe3c6 100644 --- a/javascript/ql/test/library-tests/RegExp/EscapeInString/EscapeInString.ql +++ b/javascript/ql/test/library-tests/RegExp/EscapeInString/EscapeInString.ql @@ -1,6 +1,7 @@ import javascript from StringLiteral literal, RegExpDot dot, int pos -where dot.getParent*() = literal - and pos = dot.getLocation().getStartColumn() - literal.getLocation().getStartColumn() +where + dot.getParent*() = literal and + pos = dot.getLocation().getStartColumn() - literal.getLocation().getStartColumn() select dot, literal.getRawValue().charAt(pos) diff --git a/javascript/ql/test/library-tests/RegExp/MissingUnicodeFlag/MissingUnicodeFlag.ql b/javascript/ql/test/library-tests/RegExp/MissingUnicodeFlag/MissingUnicodeFlag.ql index 80c214b41d6..e8d47d789f3 100644 --- a/javascript/ql/test/library-tests/RegExp/MissingUnicodeFlag/MissingUnicodeFlag.ql +++ b/javascript/ql/test/library-tests/RegExp/MissingUnicodeFlag/MissingUnicodeFlag.ql @@ -1,7 +1,8 @@ import javascript from RegExpLiteral literal, RegExpConstant wideConstant -where wideConstant.getLiteral() = literal and +where + wideConstant.getLiteral() = literal and not literal.getFlags().matches("%u%") and wideConstant.getValue().length() > 1 and ( diff --git a/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getPredecessor.ql b/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getPredecessor.ql index c356564dc7f..c38742da802 100644 --- a/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getPredecessor.ql +++ b/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getPredecessor.ql @@ -2,4 +2,4 @@ import javascript from RegExpTerm pred, RegExpTerm succ where pred = succ.getPredecessor() -select pred, succ \ No newline at end of file +select pred, succ diff --git a/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getSuccessor.ql b/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getSuccessor.ql index 49916f8c426..486a874c975 100644 --- a/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getSuccessor.ql +++ b/javascript/ql/test/library-tests/RegExp/predecessors_and_successors/getSuccessor.ql @@ -2,4 +2,4 @@ import javascript from RegExpTerm pred, RegExpTerm succ where succ = pred.getSuccessor() -select pred, succ \ No newline at end of file +select pred, succ diff --git a/javascript/ql/test/library-tests/TorrentLibraries/ParsedTorrent.ql b/javascript/ql/test/library-tests/TorrentLibraries/ParsedTorrent.ql index 1ad59af1d8b..eb50d8d4414 100644 --- a/javascript/ql/test/library-tests/TorrentLibraries/ParsedTorrent.ql +++ b/javascript/ql/test/library-tests/TorrentLibraries/ParsedTorrent.ql @@ -1,3 +1,3 @@ import javascript -select any(ParseTorrent::ParsedTorrent t) \ No newline at end of file +select any(ParseTorrent::ParsedTorrent t) diff --git a/javascript/ql/test/library-tests/TorrentLibraries/UserControlledTorrentInfo.ql b/javascript/ql/test/library-tests/TorrentLibraries/UserControlledTorrentInfo.ql index 85d08e75896..c18323953bf 100644 --- a/javascript/ql/test/library-tests/TorrentLibraries/UserControlledTorrentInfo.ql +++ b/javascript/ql/test/library-tests/TorrentLibraries/UserControlledTorrentInfo.ql @@ -1,3 +1,3 @@ import javascript -select any(ParseTorrent::UserControlledTorrentInfo i) \ No newline at end of file +select any(ParseTorrent::UserControlledTorrentInfo i) diff --git a/javascript/ql/test/library-tests/TorrentLibraries/parsedTorrentRef.ql b/javascript/ql/test/library-tests/TorrentLibraries/parsedTorrentRef.ql index e72bde720f3..69701796383 100644 --- a/javascript/ql/test/library-tests/TorrentLibraries/parsedTorrentRef.ql +++ b/javascript/ql/test/library-tests/TorrentLibraries/parsedTorrentRef.ql @@ -1,3 +1,3 @@ import javascript -select ParseTorrent::parsedTorrentRef() \ No newline at end of file +select ParseTorrent::parsedTorrentRef() diff --git a/javascript/ql/test/library-tests/TypeScript/CallSignatureTypes/test.ql b/javascript/ql/test/library-tests/TypeScript/CallSignatureTypes/test.ql index 57105b7de6b..03cc288f054 100644 --- a/javascript/ql/test/library-tests/TypeScript/CallSignatureTypes/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/CallSignatureTypes/test.ql @@ -13,7 +13,9 @@ query predicate test_ExprSignature(Expr expr, string type) { type = getASignatureOrElseType(expr.getType()) } -query predicate test_TypeReferenceSig(TypeReference type, SignatureKind kind, int n, CallSignatureType sig) { +query predicate test_TypeReferenceSig( + TypeReference type, SignatureKind kind, int n, CallSignatureType sig +) { sig = type.getSignature(kind, n) } @@ -21,9 +23,7 @@ query predicate test_FunctionCallSig(Function f, CallSignatureType sig) { sig = f.getCallSignature() } -query Type test_getRestParameterType(CallSignatureType sig) { - result = sig.getRestParameterType() -} +query Type test_getRestParameterType(CallSignatureType sig) { result = sig.getRestParameterType() } query Type test_getRestParameterArray(CallSignatureType sig) { result = sig.getRestParameterArrayType() diff --git a/javascript/ql/test/library-tests/TypeScript/CallSignatures/test.ql b/javascript/ql/test/library-tests/TypeScript/CallSignatures/test.ql index f3d79f91feb..e1254629fe1 100644 --- a/javascript/ql/test/library-tests/TypeScript/CallSignatures/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/CallSignatures/test.ql @@ -1,12 +1,16 @@ import javascript -query predicate test_CallSignature(CallSignature call, string declType, ASTNode body, string abstractness) { +query predicate test_CallSignature( + CallSignature call, string declType, ASTNode body, string abstractness +) { (if call.isAbstract() then abstractness = "abstract" else abstractness = "not abstract") and declType = call.getDeclaringType().describe() and body = call.getBody() } -query predicate test_IndexSignature(IndexSignature sig, string declType, ASTNode body, string abstractness) { +query predicate test_IndexSignature( + IndexSignature sig, string declType, ASTNode body, string abstractness +) { (if sig.isAbstract() then abstractness = "abstract" else abstractness = "not abstract") and declType = sig.getDeclaringType().describe() and body = sig.getBody() @@ -21,12 +25,16 @@ query predicate test_MethodOverload(MethodDeclaration method, int index, boolean if method.isOverloaded() then overloaded = true else overloaded = false } -query predicate test_FunctionCallSigOverload(FunctionCallSignature sig, int index, boolean overloaded) { +query predicate test_FunctionCallSigOverload( + FunctionCallSignature sig, int index, boolean overloaded +) { index = sig.getOverloadIndex() and if sig.isOverloaded() then overloaded = true else overloaded = false } -query predicate test_ConstructorCallSigOverload(ConstructorCallSignature sig, int index, boolean overloaded) { +query predicate test_ConstructorCallSigOverload( + ConstructorCallSignature sig, int index, boolean overloaded +) { index = sig.getOverloadIndex() and if sig.isOverloaded() then overloaded = true else overloaded = false -} \ No newline at end of file +} diff --git a/javascript/ql/test/library-tests/TypeScript/ImportMeta/test.ql b/javascript/ql/test/library-tests/TypeScript/ImportMeta/test.ql index 311aa4b4050..3753c8bab98 100644 --- a/javascript/ql/test/library-tests/TypeScript/ImportMeta/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/ImportMeta/test.ql @@ -2,4 +2,4 @@ import javascript from Expr prop where prop instanceof ImportMetaExpr -select prop \ No newline at end of file +select prop diff --git a/javascript/ql/test/library-tests/TypeScript/Namespaces/CheckBindings.ql b/javascript/ql/test/library-tests/TypeScript/Namespaces/CheckBindings.ql index a3c29fc4023..1e313974e97 100644 --- a/javascript/ql/test/library-tests/TypeScript/Namespaces/CheckBindings.ql +++ b/javascript/ql/test/library-tests/TypeScript/Namespaces/CheckBindings.ql @@ -20,8 +20,8 @@ string getNamespaceName(NamespaceDeclaration decl) { result = decl.getStmt(0).(ExprStmt).getExpr().getStringValue() or not decl.getStmt(0).(ExprStmt).getExpr() instanceof ConstantString and - result = "Namespace " + decl.getId() + " on line " + - decl.getFirstToken().getLocation().getStartLine() + result = + "Namespace " + decl.getId() + " on line " + decl.getFirstToken().getLocation().getStartLine() } from ResolveCall resolve diff --git a/javascript/ql/test/library-tests/TypeScript/PathMapping/Imports.ql b/javascript/ql/test/library-tests/TypeScript/PathMapping/Imports.ql index bd272eee884..a271f821ac4 100644 --- a/javascript/ql/test/library-tests/TypeScript/PathMapping/Imports.ql +++ b/javascript/ql/test/library-tests/TypeScript/PathMapping/Imports.ql @@ -1,8 +1,6 @@ import javascript -query predicate symbols(ASTNode astNode, CanonicalName symbol) { - ast_node_symbol(astNode, symbol) -} +query predicate symbols(ASTNode astNode, CanonicalName symbol) { ast_node_symbol(astNode, symbol) } from Import imprt select imprt, imprt.getImportedModule() diff --git a/javascript/ql/test/library-tests/TypeScript/PrivateFields/test.ql b/javascript/ql/test/library-tests/TypeScript/PrivateFields/test.ql index 27147fcb461..bf1f61039f5 100644 --- a/javascript/ql/test/library-tests/TypeScript/PrivateFields/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/PrivateFields/test.ql @@ -1,9 +1,5 @@ import javascript -query PropAccess propAccess(string name) { - result.getPropertyName() = name -} +query PropAccess propAccess(string name) { result.getPropertyName() = name } -query FieldDeclaration fieldDecl(string name) { - result.getName() = name -} +query FieldDeclaration fieldDecl(string name) { result.getName() = name } diff --git a/javascript/ql/test/library-tests/TypeScript/RegressionTests/ImportSelf/test.ql b/javascript/ql/test/library-tests/TypeScript/RegressionTests/ImportSelf/test.ql index 267a0e32c7a..d194e75f71a 100644 --- a/javascript/ql/test/library-tests/TypeScript/RegressionTests/ImportSelf/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/RegressionTests/ImportSelf/test.ql @@ -1,5 +1,5 @@ import javascript // We're mainly testing extraction succeeds, so just test that some types are extracted. - -from Expr e select e, e.getType() +from Expr e +select e, e.getType() diff --git a/javascript/ql/test/library-tests/TypeScript/RegressionTests/TraceResolution/test.ql b/javascript/ql/test/library-tests/TypeScript/RegressionTests/TraceResolution/test.ql index 9b7f3a42e02..574b7c54d4e 100644 --- a/javascript/ql/test/library-tests/TypeScript/RegressionTests/TraceResolution/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/RegressionTests/TraceResolution/test.ql @@ -1,3 +1,4 @@ import javascript -from Expr e select e, e.getType() +from Expr e +select e, e.getType() diff --git a/javascript/ql/test/library-tests/TypeScript/Tokens/PrettyPrintTokens.ql b/javascript/ql/test/library-tests/TypeScript/Tokens/PrettyPrintTokens.ql index 1f5491fc28c..c040343b2ad 100644 --- a/javascript/ql/test/library-tests/TypeScript/Tokens/PrettyPrintTokens.ql +++ b/javascript/ql/test/library-tests/TypeScript/Tokens/PrettyPrintTokens.ql @@ -7,7 +7,8 @@ Token getATokenAtLine(File file, int line) { bindingset[line] string getTokenStringAtLine(File file, int line) { - result = concat(Token tok | + result = + concat(Token tok | tok = getATokenAtLine(file, line) | tok.toString().replaceAll(" ", "~") + " " order by tok.getLocation().getStartColumn() diff --git a/javascript/ql/test/library-tests/TypeScript/TypeAliases/TypeAliases.ql b/javascript/ql/test/library-tests/TypeScript/TypeAliases/TypeAliases.ql index 60b404be4b3..e8133898ff2 100644 --- a/javascript/ql/test/library-tests/TypeScript/TypeAliases/TypeAliases.ql +++ b/javascript/ql/test/library-tests/TypeScript/TypeAliases/TypeAliases.ql @@ -3,18 +3,10 @@ import javascript from TypeAliasDeclaration decl select decl, decl.getIdentifier(), decl.getNumTypeParameter(), decl.getDefinition() -query Type rightHandSide(TypeAliasDeclaration decl) { - result = decl.getDefinition().getType() -} +query Type rightHandSide(TypeAliasDeclaration decl) { result = decl.getDefinition().getType() } -query Type getAliasedType(TypeAliasReference ref) { - result = ref.getAliasedType() -} +query Type getAliasedType(TypeAliasReference ref) { result = ref.getAliasedType() } -query Type getTypeArgument(TypeAliasReference ref, int n) { - result = ref.getTypeArgument(n) -} +query Type getTypeArgument(TypeAliasReference ref, int n) { result = ref.getTypeArgument(n) } -query Type unfold(TypeAliasReference t) { - result = t.unfold() -} +query Type unfold(TypeAliasReference t) { result = t.unfold() } diff --git a/javascript/ql/test/library-tests/TypeScript/TypeOnlyImportExport/test.ql b/javascript/ql/test/library-tests/TypeScript/TypeOnlyImportExport/test.ql index 17939b3b380..df364791e6c 100644 --- a/javascript/ql/test/library-tests/TypeScript/TypeOnlyImportExport/test.ql +++ b/javascript/ql/test/library-tests/TypeScript/TypeOnlyImportExport/test.ql @@ -1,22 +1,14 @@ import javascript -query VarRef getAVarReference(Variable v) { - result = v.getAReference() -} +query VarRef getAVarReference(Variable v) { result = v.getAReference() } -query VarRef getAnExportAccess(LocalTypeName t) { - result = t.getAnExportAccess() -} +query VarRef getAnExportAccess(LocalTypeName t) { result = t.getAnExportAccess() } -query TypeDecl getATypeDecl(LocalTypeName t) { - result = t.getADeclaration() -} +query TypeDecl getATypeDecl(LocalTypeName t) { result = t.getADeclaration() } -query Function calls(DataFlow::InvokeNode invoke) { - result = invoke.getACallee() -} +query Function calls(DataFlow::InvokeNode invoke) { result = invoke.getACallee() } query predicate exportsAs(ExportDeclaration exprt, LexicalName v, string name, string kind) { - exprt.exportsAs(v, name) and - kind = v.getDeclarationSpace() + exprt.exportsAs(v, name) and + kind = v.getDeclarationSpace() } diff --git a/javascript/ql/test/library-tests/frameworks/EventEmitter/test.ql b/javascript/ql/test/library-tests/frameworks/EventEmitter/test.ql index 846536672b1..e8de4ff4c53 100644 --- a/javascript/ql/test/library-tests/frameworks/EventEmitter/test.ql +++ b/javascript/ql/test/library-tests/frameworks/EventEmitter/test.ql @@ -1,8 +1,5 @@ import javascript query predicate taintSteps(DataFlow::Node pred, DataFlow::Node succ) { - exists(DataFlow::AdditionalFlowStep step | - step.step(pred, succ) - ) + exists(DataFlow::AdditionalFlowStep step | step.step(pred, succ)) } - diff --git a/javascript/ql/test/library-tests/frameworks/LazyCache/LazyCache.ql b/javascript/ql/test/library-tests/frameworks/LazyCache/LazyCache.ql index bca53d0f163..95c42bf99eb 100644 --- a/javascript/ql/test/library-tests/frameworks/LazyCache/LazyCache.ql +++ b/javascript/ql/test/library-tests/frameworks/LazyCache/LazyCache.ql @@ -1,5 +1,3 @@ import javascript -query DataFlow::Node moduleImport(string name) { - result = DataFlow::moduleImport(name) -} +query DataFlow::Node moduleImport(string name) { result = DataFlow::moduleImport(name) } diff --git a/javascript/ql/test/library-tests/frameworks/WebSocket/test.ql b/javascript/ql/test/library-tests/frameworks/WebSocket/test.ql index 5cb76ca7ec2..e7b9d451a55 100644 --- a/javascript/ql/test/library-tests/frameworks/WebSocket/test.ql +++ b/javascript/ql/test/library-tests/frameworks/WebSocket/test.ql @@ -14,4 +14,4 @@ query ServerWebSocket::ReceiveNode serverReceive() { any() } query predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) { any(DataFlow::AdditionalFlowStep s).step(pred, succ) -} \ No newline at end of file +} diff --git a/javascript/ql/test/library-tests/frameworks/typeahead/test.ql b/javascript/ql/test/library-tests/frameworks/typeahead/test.ql index 4b444d66bd0..29b76a1851e 100644 --- a/javascript/ql/test/library-tests/frameworks/typeahead/test.ql +++ b/javascript/ql/test/library-tests/frameworks/typeahead/test.ql @@ -1,13 +1,9 @@ import javascript -query DataFlow::Node url() { - result = any(ClientRequest r).getUrl() -} +query DataFlow::Node url() { result = any(ClientRequest r).getUrl() } query DataFlow::Node response(string responseType, boolean promise) { - result = any(ClientRequest r).getAResponseDataNode(responseType, promise) + result = any(ClientRequest r).getAResponseDataNode(responseType, promise) } -query DataFlow::Node suggestionFunction() { - result = any(Typeahead::TypeaheadSuggestionFunction t) -} \ No newline at end of file +query DataFlow::Node suggestionFunction() { result = any(Typeahead::TypeaheadSuggestionFunction t) } diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.ql b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.ql index ea18eafe350..9e4f8d9f67f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.ql +++ b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.ql @@ -6,6 +6,4 @@ where char = getAnIdentityEscapedCharacter(n, _, _) and not hasALikelyRegExpPatternMistake(n) and not char = "\n" // ignore escaped newlines in multiline strings -select n, - "The escape sequence '\\" + char + "' is equivalent to just '" + - char + "'." +select n, "The escape sequence '\\" + char + "' is equivalent to just '" + char + "'." diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected index 1256634d8e5..6f7fdeae63d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected @@ -1631,6 +1631,29 @@ nodes | normalizedPaths.js:332:19:332:32 | normalizedPath | | normalizedPaths.js:332:19:332:32 | normalizedPath | | normalizedPaths.js:332:19:332:32 | normalizedPath | +| normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | +| normalizedPaths.js:339:32:339:45 | req.query.path | +| normalizedPaths.js:339:32:339:45 | req.query.path | +| normalizedPaths.js:339:32:339:45 | req.query.path | +| normalizedPaths.js:339:32:339:45 | req.query.path | +| normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:346:19:346:22 | path | | other-fs-libraries.js:9:7:9:48 | path | | other-fs-libraries.js:9:7:9:48 | path | | other-fs-libraries.js:9:7:9:48 | path | @@ -4736,6 +4759,34 @@ edges | normalizedPaths.js:320:45:320:48 | path | normalizedPaths.js:320:23:320:49 | pathMod ... , path) | | normalizedPaths.js:320:45:320:48 | path | normalizedPaths.js:320:23:320:49 | pathMod ... , path) | | normalizedPaths.js:320:45:320:48 | path | normalizedPaths.js:320:23:320:49 | pathMod ... , path) | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:341:18:341:21 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:6:339:46 | path | normalizedPaths.js:346:19:346:22 | path | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | normalizedPaths.js:339:6:339:46 | path | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | +| normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:339:13:339:46 | pathMod ... y.path) | | other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path | | other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path | | other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path | @@ -5952,6 +6003,8 @@ edges | normalizedPaths.js:316:19:316:22 | path | normalizedPaths.js:303:13:303:26 | req.query.path | normalizedPaths.js:316:19:316:22 | path | This path depends on $@. | normalizedPaths.js:303:13:303:26 | req.query.path | a user-provided value | | normalizedPaths.js:325:19:325:32 | normalizedPath | normalizedPaths.js:303:13:303:26 | req.query.path | normalizedPaths.js:325:19:325:32 | normalizedPath | This path depends on $@. | normalizedPaths.js:303:13:303:26 | req.query.path | a user-provided value | | normalizedPaths.js:332:19:332:32 | normalizedPath | normalizedPaths.js:303:13:303:26 | req.query.path | normalizedPaths.js:332:19:332:32 | normalizedPath | This path depends on $@. | normalizedPaths.js:303:13:303:26 | req.query.path | a user-provided value | +| normalizedPaths.js:341:18:341:21 | path | normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:341:18:341:21 | path | This path depends on $@. | normalizedPaths.js:339:32:339:45 | req.query.path | a user-provided value | +| normalizedPaths.js:346:19:346:22 | path | normalizedPaths.js:339:32:339:45 | req.query.path | normalizedPaths.js:346:19:346:22 | path | This path depends on $@. | normalizedPaths.js:339:32:339:45 | req.query.path | a user-provided value | | other-fs-libraries.js:11:19:11:22 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:11:19:11:22 | path | This path depends on $@. | other-fs-libraries.js:9:24:9:30 | req.url | a user-provided value | | other-fs-libraries.js:12:27:12:30 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:12:27:12:30 | path | This path depends on $@. | other-fs-libraries.js:9:24:9:30 | req.url | a user-provided value | | other-fs-libraries.js:13:24:13:27 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:13:24:13:27 | path | This path depends on $@. | other-fs-libraries.js:9:24:9:30 | req.url | a user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js index 6a7a143f7bd..cdbdf84b966 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js @@ -334,3 +334,17 @@ app.get('/pseudo-normalizations', (req, res) => { } }); + +app.get('/yet-another-prefix', (req, res) => { + let path = pathModule.resolve(req.query.path); + + fs.readFileSync(path); // NOT OK + + var abs = pathModule.resolve(path); + + if (abs.indexOf(root) !== 0) { + fs.readFileSync(path); // NOT OK + return; + } + fs.readFileSync(path); // OK +}); \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-078/UselessUseOfCat.expected b/javascript/ql/test/query-tests/Security/CWE-078/UselessUseOfCat.expected new file mode 100644 index 00000000000..4f2dae3d7b1 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-078/UselessUseOfCat.expected @@ -0,0 +1,112 @@ +readFile +| uselesscat.js:10:1:10:43 | exec("c ... ut) {}) | fs.readFile("foo/bar", function(err, out) {...}) | +| uselesscat.js:12:1:14:2 | exec("c ... ut);\\n}) | fs.readFile("/proc/" + id + "/status", function(err, out) {...}) | +| uselesscat.js:16:1:16:29 | execSyn ... uinfo') | fs.readFileSync("/proc/cpuinfo") | +| uselesscat.js:18:1:18:26 | execSyn ... path}`) | fs.readFileSync(newpath) | +| uselesscat.js:32:1:32:34 | execSyn ... path}`) | fs.readFileSync(`foo/bar/${newpath}`) | +| uselesscat.js:34:1:34:54 | execSyn ... utf8'}) | fs.readFileSync(`foo/bar/${newpath}`, {encoding: 'utf8'}) | +| uselesscat.js:51:9:51:31 | execSyn ... + file) | fs.readFileSync(file) | +| uselesscat.js:59:1:62:2 | execFil ... ut);\\n}) | fs.readFile("pom.xml", function(error, stdout, stderr) {...}) | +| uselesscat.js:69:1:72:2 | execFil ... ut);\\n}) | fs.readFile("pom.xml", {encoding: 'utf8'}, function(error, stdout, stderr) {...}) | +| uselesscat.js:74:1:74:60 | execFil ... utf8'}) | fs.readFileSync("pom.xml", {encoding: 'utf8'}) | +| uselesscat.js:76:1:76:39 | execFil ... xml' ]) | fs.readFileSync("pom.xml") | +| uselesscat.js:79:1:79:46 | execFil ... opts) | fs.readFileSync("pom.xml", opts) | +| uselesscat.js:82:1:82:90 | execFil ... String) | fs.readFileSync("pom.xml", anOptsFileNameThatIsTooLongToBePrintedByToString) | +| uselesscat.js:84:1:84:115 | execFil ... ring'}) | fs.readFileSync("pom.xml", ...) | +| uselesscat.js:86:1:86:75 | execFil ... utf8'}) | fs.readFileSync("foo/" + newPath + "bar", {encoding: 'utf8'}) | +| uselesscat.js:88:1:88:35 | execSyn ... + foo) | fs.readFileSync("/proc/cpuinfo" + foo) | +| uselesscat.js:90:1:90:50 | execFil ... th}` ]) | fs.readFileSync(`foo/bar/${newpath}`) | +| uselesscat.js:94:1:94:43 | exec("c ... ut) {}) | fs.readFile("foo/bar", function(err, out) {...}) | +| uselesscat.js:96:1:96:53 | exec("c ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:98:1:98:55 | exec("c ... h(out)) | fs.readFile("foo/bar", (err, out) => ...) | +| uselesscat.js:121:12:121:64 | exec("c ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:127:14:127:66 | exec("c ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:136:17:138:2 | execSyn ... tf8'\\n}) | fs.readFileSync("/etc/dnsmasq.conf", ...) | +| uselesscat.js:146:1:146:61 | shelljs ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:147:1:147:47 | shelljs ... utf8'}) | fs.readFile("foo/bar", {encoding: 'utf8'}) | +| uselesscat.js:148:1:148:81 | shelljs ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:151:1:151:48 | cspawn( ... tf8' }) | fs.readFile("foo/bar", { encoding: 'utf8' }) | +| uselesscat.js:152:1:152:82 | cspawn( ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:153:1:153:60 | cspawn( ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:154:1:154:26 | cspawn( ... /bar']) | fs.readFile("foo/bar") | +| uselesscat.js:158:16:158:46 | cspawn. ... /bar']) | fs.readFileSync("foo/bar") | +| uselesscat.js:159:16:159:68 | cspawn. ... tf8' }) | fs.readFileSync("foo/bar", { encoding: 'utf8' }) | +| uselesscat.js:162:1:162:56 | execmod ... (out)}) | fs.readFile("foo/bar", (err, out) => {...}) | +| uselesscat.js:163:1:163:42 | execmod ... utf8'}) | fs.readFile("foo/bar") | +| uselesscat.js:164:1:164:76 | execmod ... (out)}) | fs.readFile("foo/bar", {encoding: 'utf8'}, (err, out) => {...}) | +syncCommand +| child_process-test.js:9:5:9:22 | cp.execSync("foo") | +| child_process-test.js:11:5:11:26 | cp.exec ... ("foo") | +| child_process-test.js:13:5:13:23 | cp.spawnSync("foo") | +| child_process-test.js:18:5:18:20 | cp.execSync(cmd) | +| child_process-test.js:20:5:20:24 | cp.execFileSync(cmd) | +| child_process-test.js:22:5:22:21 | cp.spawnSync(cmd) | +| command-line-parameter-command-injection.js:11:2:11:21 | cp.execSync(args[0]) | +| command-line-parameter-command-injection.js:12:2:12:33 | cp.exec ... rgs[0]) | +| command-line-parameter-command-injection.js:15:2:15:26 | cp.exec ... rgs[0]) | +| command-line-parameter-command-injection.js:16:2:16:38 | cp.exec ... rgs[0]) | +| command-line-parameter-command-injection.js:19:2:19:18 | cp.execSync(arg0) | +| command-line-parameter-command-injection.js:20:2:20:30 | cp.exec ... + arg0) | +| command-line-parameter-command-injection.js:26:2:26:51 | cp.exec ... tion"`) | +| command-line-parameter-command-injection.js:27:2:27:58 | cp.exec ... tion"`) | +| other.js:7:5:7:36 | require ... nc(cmd) | +| other.js:9:5:9:35 | require ... nc(cmd) | +| other.js:12:5:12:30 | require ... nc(cmd) | +| third-party-command-injection.js:6:9:6:28 | cp.execSync(command) | +| tst_shell-command-injection-from-environment.js:4:2:4:62 | cp.exec ... emp")]) | +| tst_shell-command-injection-from-environment.js:5:2:5:54 | cp.exec ... temp")) | +| uselesscat.js:16:1:16:29 | execSyn ... uinfo') | +| uselesscat.js:18:1:18:26 | execSyn ... path}`) | +| uselesscat.js:20:1:20:36 | execSyn ... wc -l') | +| uselesscat.js:22:1:22:38 | execSyn ... o/bar') | +| uselesscat.js:24:1:24:35 | execSyn ... o/bar`) | +| uselesscat.js:28:1:28:39 | execSyn ... 1000}) | +| uselesscat.js:32:1:32:34 | execSyn ... path}`) | +| uselesscat.js:34:1:34:54 | execSyn ... utf8'}) | +| uselesscat.js:36:1:36:77 | execSyn ... utf8'}) | +| uselesscat.js:38:1:38:43 | execSyn ... r/baz') | +| uselesscat.js:40:1:40:40 | execSyn ... path}`) | +| uselesscat.js:42:1:42:47 | execSyn ... File}`) | +| uselesscat.js:44:1:44:34 | execSyn ... ' ')}`) | +| uselesscat.js:48:1:48:41 | execSyn ... tool}`) | +| uselesscat.js:51:9:51:31 | execSyn ... + file) | +| uselesscat.js:54:1:54:39 | execSyn ... + "'") | +| uselesscat.js:74:1:74:60 | execFil ... utf8'}) | +| uselesscat.js:76:1:76:39 | execFil ... xml' ]) | +| uselesscat.js:79:1:79:46 | execFil ... opts) | +| uselesscat.js:82:1:82:90 | execFil ... String) | +| uselesscat.js:84:1:84:115 | execFil ... ring'}) | +| uselesscat.js:86:1:86:75 | execFil ... utf8'}) | +| uselesscat.js:88:1:88:35 | execSyn ... + foo) | +| uselesscat.js:90:1:90:50 | execFil ... th}` ]) | +| uselesscat.js:92:1:92:46 | execFil ... th}` ]) | +| uselesscat.js:100:1:100:56 | execFil ... ptions) | +| uselesscat.js:104:1:104:31 | execFil ... cat` ]) | +| uselesscat.js:136:17:138:2 | execSyn ... tf8'\\n}) | +| uselesscat.js:158:16:158:46 | cspawn. ... /bar']) | +| uselesscat.js:159:16:159:68 | cspawn. ... tf8' }) | +options +| child_process-test.js:53:5:53:59 | cp.spaw ... cmd])) | child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) | +| child_process-test.js:54:5:54:50 | cp.spaw ... t(cmd)) | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) | +| child_process-test.js:64:3:64:21 | cp.spawn(cmd, args) | child_process-test.js:64:17:64:20 | args | +| uselesscat.js:28:1:28:39 | execSyn ... 1000}) | uselesscat.js:28:28:28:38 | {uid: 1000} | +| uselesscat.js:30:1:30:64 | exec('c ... t) { }) | uselesscat.js:30:26:30:38 | { cwd: './' } | +| uselesscat.js:34:1:34:54 | execSyn ... utf8'}) | uselesscat.js:34:36:34:53 | {encoding: 'utf8'} | +| uselesscat.js:36:1:36:77 | execSyn ... utf8'}) | uselesscat.js:36:36:36:76 | { uid: ... 'utf8'} | +| uselesscat.js:69:1:72:2 | execFil ... ut);\\n}) | uselesscat.js:69:38:69:55 | {encoding: 'utf8'} | +| uselesscat.js:74:1:74:60 | execFil ... utf8'}) | uselesscat.js:74:42:74:59 | {encoding: 'utf8'} | +| uselesscat.js:79:1:79:46 | execFil ... opts) | uselesscat.js:79:42:79:45 | opts | +| uselesscat.js:82:1:82:90 | execFil ... String) | uselesscat.js:82:42:82:89 | anOptsF ... oString | +| uselesscat.js:84:1:84:115 | execFil ... ring'}) | uselesscat.js:84:42:84:114 | {encodi ... tring'} | +| uselesscat.js:86:1:86:75 | execFil ... utf8'}) | uselesscat.js:86:57:86:74 | {encoding: 'utf8'} | +| uselesscat.js:100:1:100:56 | execFil ... ptions) | uselesscat.js:100:42:100:55 | unknownOptions | +| uselesscat.js:111:1:111:51 | spawn(' ... it'] }) | uselesscat.js:111:14:111:50 | { stdio ... rit'] } | +| uselesscat.js:136:17:138:2 | execSyn ... tf8'\\n}) | uselesscat.js:136:51:138:1 | { // NO ... utf8'\\n} | +| uselesscat.js:147:1:147:47 | shelljs ... utf8'}) | uselesscat.js:147:29:147:46 | {encoding: 'utf8'} | +| uselesscat.js:151:1:151:48 | cspawn( ... tf8' }) | uselesscat.js:151:28:151:47 | { encoding: 'utf8' } | +| uselesscat.js:156:1:156:35 | cspawn( ... tf8' }) | uselesscat.js:156:15:156:34 | { encoding: 'utf8' } | +| uselesscat.js:159:16:159:68 | cspawn. ... tf8' }) | uselesscat.js:159:48:159:67 | { encoding: 'utf8' } | +| uselesscat.js:164:1:164:76 | execmod ... (out)}) | uselesscat.js:164:24:164:41 | {encoding: 'utf8'} | +#select +| False negative | uselesscat.js:54:42:54:69 | // NOT ... lagged] | +| False positive | uselesscat.js:44:37:44:85 | // OK [ ... le read | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/UselessUseOfCat.ql b/javascript/ql/test/query-tests/Security/CWE-078/UselessUseOfCat.ql new file mode 100644 index 00000000000..a70f53016bd --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-078/UselessUseOfCat.ql @@ -0,0 +1,28 @@ +import javascript +import semmle.javascript.security.UselessUseOfCat + +from LineComment comment, string msg +where + comment.getFile().getAbsolutePath().regexpMatch(".*/uselesscat.js") and + ( + comment.getText().regexpMatch(".*NOT OK.*") and + not any(UselessCat cat).asExpr().getLocation().getStartLine() = + comment.getLocation().getStartLine() and + msg = "False negative" + or + comment.getText().regexpMatch(".* OK.*") and + not comment.getText().regexpMatch(".*NOT OK.*") and + any(UselessCat cat).asExpr().getLocation().getStartLine() = comment.getLocation().getStartLine() and + msg = "False positive" + ) +select msg, comment + +query string readFile(UselessCat cat) { result = PrettyPrintCatCall::createReadFileCall(cat) } + +query SystemCommandExecution syncCommand() { + result.isSync() +} + +query DataFlow::Node options(SystemCommandExecution sys) { + result = sys.getOptionsArg() +} \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-078/uselesscat.js b/javascript/ql/test/query-tests/Security/CWE-078/uselesscat.js new file mode 100644 index 00000000000..ace4784b380 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-078/uselesscat.js @@ -0,0 +1,166 @@ +var express = require('express'); +var child_process = require('child_process'); +var execSync = child_process.execSync; +var exec = child_process.exec; +var spawn = child_process.spawn; +var spawnSync = child_process.spawnSync; +var fs = require('fs'); +var app = express(); + +exec("cat foo/bar", function (err, out) {}); // NOT OK + +exec("cat /proc/" + id + "/status", function (err, out) { // NOT OK + console.log(out); +}); + +execSync('cat /proc/cpuinfo').toString(); // NOT OK. + +execSync(`cat ${newpath}`) // NOT OK + +execSync('cat package.json | wc -l'); // OK - pipes! + +execSync('cat /proc/cpuinfo /foo/bar').toString(); // OK multiple files. + +execSync(`cat ${newpath} /foo/bar`).toString(); // OK multiple files. + +exec(`cat ${newpath} | grep foo`, function (err, out) { }) // OK - pipes + +execSync(`cat ${newpath}`, {uid: 1000}) // OK - non trivial options + +exec('cat *.js | wc -l', { cwd: './' }, function (err, out) { }); // OK - wildcard and pipes + +execSync(`cat foo/bar/${newpath}`); // NOT OK ("encoding" is used EXACTLY the same way in fs.readFileSync) + +execSync(`cat foo/bar/${newpath}`, {encoding: 'utf8'}); // NOT OK ("encoding" is used EXACTLY the same way in fs.readFileSync) + +execSync("/bin/cat /proc/cpuinfo", { uid: 1000, gid: 1000, encoding: 'utf8'}); // OK (fs.readFileSync cannot emulate uid / gid)) + +execSync('cat /proc/cpuinfo > foo/bar/baz').toString(); // OK. + +execSync(`cat ${newpath} > ${destpath}`).toString(); // OK. + +execSync(`cat ${files.join(' ')} > ${outFile}`); // OK + +execSync(`cat ${files.join(' ')}`); // OK [but flagged] - not just a simple file read + +exec("cat /proc/cpuinfo | grep name"); // OK - pipes + +execSync(`cat ${newpath} | ${othertool}`); // OK - pipes + +function cat(file) { + return execSync('cat ' + file).toString(); // NOT OK +} + +execSync("sh -c 'cat " + newpath + "'"); // NOT OK. [but not flagged] + +var execFile = child_process.execFile; +var execFileSync = child_process.execFileSync; + +execFile('/bin/cat', [ 'pom.xml' ], function(error, stdout, stderr ) { // NOT OK + // Not using stderr + console.log(stdout); +}); + +execFile('/bin/cat', [ 'pom.xml' ], function(error, stdout, stderr ) { // OK. - stderr is used. + console.log(stderr); +}); + + +execFile('/bin/cat', [ 'pom.xml' ], {encoding: 'utf8'}, function(error, stdout, stderr ) { // NOT OK + // Not using stderr + console.log(stdout); +}); + +execFileSync('/bin/cat', [ 'pom.xml' ], {encoding: 'utf8'}); // NOT OK + +execFileSync('/bin/cat', [ 'pom.xml' ]); // NOT OK + +var opts = {encoding: 'utf8'}; +execFileSync('/bin/cat', [ 'pom.xml' ], opts); // NOT OK + +var anOptsFileNameThatIsTooLongToBePrintedByToString = {encoding: 'utf8'}; +execFileSync('/bin/cat', [ 'pom.xml' ], anOptsFileNameThatIsTooLongToBePrintedByToString); // NOT OK + +execFileSync('/bin/cat', [ 'pom.xml' ], {encoding: 'someEncodingValueThatIsCompletelyBogusAndTooLongForToString'}); // NOT OK + +execFileSync('/bin/cat', [ "foo/" + newPath + "bar" ], {encoding: 'utf8'}); // NOT OK + +execSync('cat /proc/cpuinfo' + foo).toString(); // NOT OK. + +execFileSync('/bin/cat', [ `foo/bar/${newpath}` ]); // NOT OK + +execFileSync('node', [ `foo/bar/${newpath}` ]); // OK - not a call to cat + +exec("cat foo/bar", function (err, out) {}); // NOT OK + +exec("cat foo/bar", (err, out) => {console.log(out)}); // NOT OK + +exec("cat foo/bar", (err, out) => doSomethingWith(out)); // NOT OK + +execFileSync('/bin/cat', [ 'pom.xml' ], unknownOptions); // OK - unknown options. + +exec("node foo/bar", (err, out) => doSomethingWith(out)); // OK - Not a call to cat + +execFileSync('node', [ `cat` ]); // OK - not a call to cat + +exec("cat foo/bar&", function (err, out) {}); // OK - contains & +exec("cat foo/bar,", function (err, out) {}); // OK - contains , +exec("cat foo/bar$", function (err, out) {}); // OK - contains $ +exec("cat foo/bar`", function (err, out) {}); // OK - contains ` + +spawn('cat', { stdio: ['pipe', stdin, 'inherit'] }); // OK - Non trivial use. (But weird API use.) + +(function () { + const cat = spawn('cat', [filename]); // OK - non trivial use. + cat.stdout.on('data', (data) => { + res.write(data); + }); + cat.stdout.on('end', () => res.end()); +})(); + +var dead = exec("cat foo/bar", (err, out) => {console.log(out)}); // NOT OK + +var notDead = exec("cat foo/bar", (err, out) => {console.log(out)}); // OK +console.log(notDead); + +(function () { + var dead = exec("cat foo/bar", (err, out) => {console.log(out)}); // NOT OK + + someCall( + exec("cat foo/bar", (err, out) => {console.log(out)}) // OK - non-trivial use of returned proccess. + ); + + return exec("cat foo/bar", (err, out) => {console.log(out)}); // OK - non-trivial use of returned proccess. +})(); + +const stdout2 = execSync('cat /etc/dnsmasq.conf', { // NOT OK. + encoding: 'utf8' +}); + +exec('/bin/cat', function (e, s) {}); // OK + +spawn("cat") // OK + + +var shelljs = require("shelljs"); +shelljs.exec("cat foo/bar", (err, out) => {console.log(out)}); // NOT OK +shelljs.exec("cat foo/bar", {encoding: 'utf8'}); // NOT OK +shelljs.exec("cat foo/bar", {encoding: 'utf8'}, (err, out) => {console.log(out)}); // NOT OK + +let cspawn = require('cross-spawn'); +cspawn('cat', ['foo/bar'], { encoding: 'utf8' }); // NOT OK +cspawn('cat', ['foo/bar'], { encoding: 'utf8' }, (err, out) => {console.log(out)}); // NOT OK +cspawn('cat', ['foo/bar'], (err, out) => {console.log(out)}); // NOT OK +cspawn('cat', ['foo/bar']); // NOT OK +cspawn('cat', (err, out) => {console.log(out)}); // OK +cspawn('cat', { encoding: 'utf8' }); // OK + +let myResult = cspawn.sync('cat', ['foo/bar']); // NOT OK +let myResult = cspawn.sync('cat', ['foo/bar'], { encoding: 'utf8' }); // NOT OK + +var execmod = require('exec'); +execmod("cat foo/bar", (err, out) => {console.log(out)}); // NOT OK +execmod("cat foo/bar", {encoding: 'utf8'}); // NOT OK +execmod("cat foo/bar", {encoding: 'utf8'}, (err, out) => {console.log(out)}); // NOT OK + + \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-079/XssWithAdditionalSources.ql b/javascript/ql/test/query-tests/Security/CWE-079/XssWithAdditionalSources.ql index 543c2537253..96dee32082b 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/XssWithAdditionalSources.ql +++ b/javascript/ql/test/query-tests/Security/CWE-079/XssWithAdditionalSources.ql @@ -16,7 +16,6 @@ import semmle.javascript.security.dataflow.DomBasedXss::DomBasedXss import DataFlow::PathGraph import semmle.javascript.heuristics.AdditionalSources - from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource select sink.getNode(), source, sink, diff --git a/python/ql/src/Expressions/UseofApply.ql b/python/ql/src/Expressions/UseofApply.ql index 8f913e6980d..7a0d72b43cf 100644 --- a/python/ql/src/Expressions/UseofApply.ql +++ b/python/ql/src/Expressions/UseofApply.ql @@ -10,8 +10,8 @@ */ import python +private import semmle.python.types.Builtins from CallNode call, ControlFlowNode func -where -major_version() = 2 and call.getFunction() = func and func.refersTo(Object::builtin("apply")) +where major_version() = 2 and call.getFunction() = func and func.pointsTo(Value::named("apply")) select call, "Call to the obsolete builtin function 'apply'." diff --git a/python/ql/test/2/query-tests/Expressions/UseofApply.expected b/python/ql/test/2/query-tests/Expressions/UseofApply.expected index 082296ef25c..aac8fbcda88 100644 --- a/python/ql/test/2/query-tests/Expressions/UseofApply.expected +++ b/python/ql/test/2/query-tests/Expressions/UseofApply.expected @@ -1 +1,2 @@ +| UseofApply.py:19:3:19:17 | ControlFlowNode for apply() | Call to the obsolete builtin function 'apply'. | | expressions_test.py:3:5:3:21 | ControlFlowNode for apply() | Call to the obsolete builtin function 'apply'. | diff --git a/python/ql/test/2/query-tests/Expressions/UseofApply.py b/python/ql/test/2/query-tests/Expressions/UseofApply.py new file mode 100644 index 00000000000..9109636f99e --- /dev/null +++ b/python/ql/test/2/query-tests/Expressions/UseofApply.py @@ -0,0 +1,30 @@ +#### UseofApply.ql + +# Use of the builtin function `apply` is generally considered bad now that the +# ability to destructure lists of arguments is possible, but we should not flag +# cases where the function is merely named `apply` rather than being the actual +# builtin `apply` function. + +def useofapply(): + + def foo(): + pass + + + + # Positive Cases + + # This use of `apply` is a reference to the builtin function and so SHOULD be + # caught by the query. + apply(foo, [1]) + + + + # Negative Cases + + # This use of `apply` is a reference to the locally defined function inside of + # `local`, and so SHOULD NOT be caught by the query. + def local(): + def apply(f): + pass + apply(foo)([1]) diff --git a/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.expected b/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.py b/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.py new file mode 100644 index 00000000000..9109636f99e --- /dev/null +++ b/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.py @@ -0,0 +1,30 @@ +#### UseofApply.ql + +# Use of the builtin function `apply` is generally considered bad now that the +# ability to destructure lists of arguments is possible, but we should not flag +# cases where the function is merely named `apply` rather than being the actual +# builtin `apply` function. + +def useofapply(): + + def foo(): + pass + + + + # Positive Cases + + # This use of `apply` is a reference to the builtin function and so SHOULD be + # caught by the query. + apply(foo, [1]) + + + + # Negative Cases + + # This use of `apply` is a reference to the locally defined function inside of + # `local`, and so SHOULD NOT be caught by the query. + def local(): + def apply(f): + pass + apply(foo)([1]) diff --git a/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.qlref b/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.qlref new file mode 100644 index 00000000000..abf684e3918 --- /dev/null +++ b/python/ql/test/3/query-tests/Expressions/UseofApply/UseofApply.qlref @@ -0,0 +1 @@ +Expressions/UseofApply.ql diff --git a/python/ql/test/query-tests/Expressions/general/expressions_test.py b/python/ql/test/query-tests/Expressions/general/expressions_test.py index 1d802f1bfd6..5e07b58e204 100644 --- a/python/ql/test/query-tests/Expressions/general/expressions_test.py +++ b/python/ql/test/query-tests/Expressions/general/expressions_test.py @@ -242,3 +242,40 @@ from types import MappingProxyType def mpt_arg(d=MappingProxyType({})): return 1 in d + + + + + + + +#### UseofApply.ql + +# Use of the builtin function `apply` is generally considered bad now that the +# ability to destructure lists of arguments is possible, but we should not flag +# cases where the function is merely named `apply` rather than being the actual +# builtin `apply` function. + +def useofapply(): + + def foo(): + pass + + + + # Positive Cases + + # This use of `apply` is a reference to the builtin function and so SHOULD be + # caught by the query. + apply(foo, [1]) + + + + # Negative Cases + + # This use of `apply` is a reference to the locally defined function inside of + # `local`, and so SHOULD NOT be caught by the query. + def local(): + def apply(f): + pass + apply(foo)([1])