diff --git a/powershell/ql/lib/semmle/code/powershell/ApiGraphs.qll b/powershell/ql/lib/semmle/code/powershell/ApiGraphs.qll index c7a4aa152a7..78cd43b5e8c 100644 --- a/powershell/ql/lib/semmle/code/powershell/ApiGraphs.qll +++ b/powershell/ql/lib/semmle/code/powershell/ApiGraphs.qll @@ -577,7 +577,7 @@ module API { ) or exists(MemberExprReadAccess read | - read.getMemberName().toLowerCase() = name and + read.getLowerCaseMemberName().toLowerCase() = name and pred = getForwardEndNode(getALocalSourceStrict(getNodeFromExpr(read.getQualifier()))) and succ = getForwardStartNode(getNodeFromExpr(read)) ) diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll index b68cc5f6e49..e1334b1dbdc 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll @@ -24,6 +24,10 @@ class ConstantValue extends TConstantValue { /** Gets the value of this consant. */ string getValue() { none() } + bindingset[s] + pragma[inline_late] + final predicate stringMatches(string s) { this.asString().toLowerCase() = s.toLowerCase() } + /** Gets the integer value of this constant, if any. */ int asInt() { none() } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Function.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Function.qll index e640fdbaaa4..42e16d921c6 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Function.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Function.qll @@ -1,7 +1,7 @@ private import AstImport class Function extends FunctionBase, TFunction { - final override string getName() { any(Synthesis s).functionName(this, result) } + final override string getLowerCaseName() { any(Synthesis s).functionName(this, result) } final override ScriptBlock getBody() { any(Synthesis s).functionScriptBlock(this, result) } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/FunctionBase.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/FunctionBase.qll index f5e58cf80b0..e238e668b64 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/FunctionBase.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/FunctionBase.qll @@ -2,11 +2,13 @@ private import AstImport private import semmle.code.powershell.controlflow.BasicBlocks class FunctionBase extends Ast, TFunctionBase { - final override string toString() { result = this.getName() } + final override string toString() { result = this.getLowerCaseName() } - string getName() { none() } + string getLowerCaseName() { none() } - final predicate hasName(string name) { name = this.getName() } + bindingset[name] + pragma[inline_late] + predicate nameMatches(string name) { this.getLowerCaseName() = name.toLowerCase() } ScriptBlock getBody() { none() } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Member.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Member.qll index e650ae94754..8f38f0e6c86 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Member.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Member.qll @@ -1,12 +1,16 @@ private import AstImport class Member extends Ast, TMember { - string getName() { - result = getRawAst(this).(Raw::Member).getName() + string getLowerCaseName() { + result = getRawAst(this).(Raw::Member).getName().toLowerCase() or any(Synthesis s).memberName(this, result) } + bindingset[name] + pragma[inline_late] + predicate memberNameMatches(string name) { this.getLowerCaseName() = name.toLowerCase() } + Type getDeclaringType() { result.getAMember() = this } final Attribute getAttribute(int i) { diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/MemberExpr.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/MemberExpr.qll index 547385a52f7..0060f891ecb 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/MemberExpr.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/MemberExpr.qll @@ -28,19 +28,29 @@ class MemberExpr extends Expr, TMemberExpr { } /** Gets the name of the member being looked up, if any. */ - string getMemberName() { + string getLowerCaseMemberName() { result = - getRawAst(this).(Raw::MemberExpr).getMember().(Raw::StringConstExpr).getValue().getValue() + getRawAst(this) + .(Raw::MemberExpr) + .getMember() + .(Raw::StringConstExpr) + .getValue() + .getValue() + .toLowerCase() } + bindingset[name] + pragma[inline_late] + predicate memberNameMatches(string name) { this.getLowerCaseMemberName() = name.toLowerCase() } + predicate isNullConditional() { getRawAst(this).(Raw::MemberExpr).isNullConditional() } predicate isStatic() { getRawAst(this).(Raw::MemberExpr).isStatic() } final override string toString() { - result = this.getMemberName() + result = this.getLowerCaseMemberName() or - not exists(this.getMemberName()) and + not exists(this.getLowerCaseMemberName()) and result = "..." } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Method.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Method.qll index ebe47dd9b9d..8ce4df97f4d 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Method.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Method.qll @@ -1,7 +1,7 @@ private import AstImport class Method extends Member, FunctionBase, TMethod { - final override string getName() { result = Member.super.getName() } + final override string getLowerCaseName() { result = Member.super.getLowerCaseName() } final override ScriptBlock getBody() { exists(Raw::Ast r | r = getRawAst(this) | @@ -24,9 +24,7 @@ class Method extends Member, FunctionBase, TMethod { predicate isConstructor() { getRawAst(this).(Raw::Method).isConstructor() } - ThisParameter getThisParameter() { - result.getFunction() = this - } + ThisParameter getThisParameter() { result.getFunction() = this } } /** A constructor definition. */ diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/PropertyMember.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/PropertyMember.qll index 29dac5861ac..247e10dab2d 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/PropertyMember.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/PropertyMember.qll @@ -1,7 +1,7 @@ private import AstImport class PropertyMember extends Member, TPropertyMember { - final override string getName() { result = getRawAst(this).(Raw::PropertyMember).getName() } + final override string getLowerCaseName() { result = getRawAst(this).(Raw::PropertyMember).getName().toLowerCase() } - final override string toString() { result = this.getName() } + final override string toString() { result = this.getLowerCaseName() } } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Type.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Type.qll index 5f7c739c9d4..cc0195adbc4 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Type.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Type.qll @@ -1,21 +1,21 @@ private import AstImport class Type extends Ast, TTypeSynth { - override string toString() { result = this.getName() } + override string toString() { result = this.getLowerCaseName() } Member getMember(int i) { any(Synthesis s).typeMember(this, i, result) } - string getName() { any(Synthesis s).typeName(this, result) } + string getLowerCaseName() { any(Synthesis s).typeName(this, result) } Member getAMember() { result = this.getMember(_) } - Method getMethod(string name) { result = this.getAMember() and result.getName() = name } + Method getMethod(string name) { result = this.getAMember() and result.getLowerCaseName() = name } Method getAMethod() { result = this.getMethod(_) } Constructor getAConstructor() { result = this.getAMethod() and - result.getName() = this.getName() + result.getLowerCaseName() = this.getLowerCaseName() } TypeConstraint getBaseType(int i) { none() } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/TypeDefinitionStmt.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/TypeDefinitionStmt.qll index e630c272814..88e080266ba 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/TypeDefinitionStmt.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/TypeDefinitionStmt.qll @@ -1,9 +1,9 @@ private import AstImport class TypeDefinitionStmt extends Stmt, TTypeDefinitionStmt { - string getName() { result = getRawAst(this).(Raw::TypeStmt).getName() } + string getLowerCaseName() { result = getRawAst(this).(Raw::TypeStmt).getName().toLowerCase() } - override string toString() { result = this.getName() } + override string toString() { result = this.getLowerCaseName() } Member getMember(int i) { exists(ChildIndex index, Raw::Ast r | index = typeStmtMember(i) and r = getRawAst(this) | @@ -24,7 +24,7 @@ class TypeDefinitionStmt extends Stmt, TTypeDefinitionStmt { Constructor getAConstructor() { result = this.getAMethod() and - result.getName() = this.getName() + result.getLowerCaseName() = this.getLowerCaseName() } TypeConstraint getBaseType(int i) { @@ -38,7 +38,7 @@ class TypeDefinitionStmt extends Stmt, TTypeDefinitionStmt { TypeConstraint getABaseType() { result = this.getBaseType(_) } - TypeDefinitionStmt getASubtype() { result.getABaseType().getName() = this.getName() } + TypeDefinitionStmt getASubtype() { result.getABaseType().getName() = this.getLowerCaseName() } Type getType() { synthChild(getRawAst(this), typeDefType(), result) } diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll index cf04bead05d..dcab50e1f02 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -639,7 +639,11 @@ module ExprNodes { ExprCfgNode getMemberExpr() { e.hasCfgChild(e.getMemberExpr(), this, result) } - string getMemberName() { result = e.getMemberName() } + string getLowerCaseMemberName() { result = e.getLowerCaseMemberName() } + + bindingset[name] + pragma[inline_late] + predicate memberNameMatches(string name) { this.getLowerCaseMemberName() = name.toLowerCase() } predicate isStatic() { e.isStatic() } } @@ -1504,7 +1508,7 @@ module StmtNodes { Type getType() { result = s.getType() } - string getName() { result = s.getName() } + string getLowerCaseName() { result = s.getLowerCaseName() } } private class FunctionDefinitionChildMapping extends NonExprChildMapping, FunctionDefinitionStmt { diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowDispatch.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowDispatch.qll index 57e3f539094..863bea7779c 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowDispatch.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowDispatch.qll @@ -210,10 +210,10 @@ Node trackInstance(string typename, boolean exact) { } private Type getTypeWithName(string s, boolean exact) { - result.getName() = s and + result.getLowerCaseName() = s and exact = true or - result.getASubtype+().getName() = s and + result.getASubtype+().getLowerCaseName() = s and exact = false } diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll index bd0aa428094..72ad1786ca3 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll @@ -353,9 +353,9 @@ private module Cached { cached newtype TContent = TFieldContent(string name) { - name = any(PropertyMember member).getName() + name = any(PropertyMember member).getLowerCaseName() or - name = any(MemberExpr me).getMemberName() + name = any(MemberExpr me).getLowerCaseMemberName() } or // A known map key TKnownKeyContent(ConstantValue cv) { exists(cv.asString()) } or @@ -892,7 +892,7 @@ predicate storeStep(Node node1, ContentSet c, Node node2) { exists(CfgNodes::ExprNodes::MemberExprWriteAccessCfgNode var, Content::FieldContent fc | node2.(PostUpdateNode).getPreUpdateNode().asExpr() = var.getQualifier() and node1.asExpr() = var.getAssignStmt().getRightHandSide() and - fc.getName() = var.getMemberName() and + fc.getLowerCaseName() = var.getLowerCaseMemberName() and c.isSingleton(fc) ) or @@ -961,7 +961,7 @@ predicate readStep(Node node1, ContentSet c, Node node2) { exists(CfgNodes::ExprNodes::MemberExprReadAccessCfgNode var, Content::FieldContent fc | node2.asExpr() = var and node1.asExpr() = var.getQualifier() and - fc.getName() = var.getMemberName() and + fc.getLowerCaseName() = var.getLowerCaseMemberName() and c.isSingleton(fc) ) or @@ -1310,7 +1310,7 @@ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) exists(kind) and exists(FunctionBase f | f.getBody() = c.asCfgScope() and - creation.asExpr().(CmdName).getName() = f.getName() + creation.asExpr().(CmdName).getName() = f.getLowerCaseName() ) } diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll index 4d9681761f9..1d444a03ab5 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPublic.qll @@ -301,7 +301,7 @@ module Content { FieldContent() { this = TFieldContent(name) } /** Gets the name of the field. */ - string getName() { result = name } + string getLowerCaseName() { result = name } override string toString() { result = name } } @@ -527,6 +527,9 @@ class CallNode extends ExprNode { /** Gets the argument with the name `name`, if any. */ Node getNamedArgument(string name) { result.asExpr() = call.getNamedArgument(name) } + /** Holds if an argument with name `name` is provided to this call. */ + predicate hasNamedArgument(string name) { call.hasNamedArgument(name) } + /** * Gets any argument of this call. * diff --git a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll new file mode 100644 index 00000000000..a7588ce2d37 --- /dev/null +++ b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll @@ -0,0 +1,78 @@ +/** + * Provides default sources, sinks and sanitizers for reasoning about + * SQL-injection vulnerabilities, as well as extension points for + * adding your own. + */ + +private import semmle.code.powershell.dataflow.DataFlow +import semmle.code.powershell.ApiGraphs +private import semmle.code.powershell.dataflow.flowsources.FlowSources +private import semmle.code.powershell.Cfg + +module SqlInjection { + /** + * A data flow source for SQL-injection vulnerabilities. + */ + abstract class Source extends DataFlow::Node { + /** Gets a string that describes the type of this flow source. */ + abstract string getSourceType(); + } + + /** + * A data flow sink for SQL-injection vulnerabilities. + */ + abstract class Sink extends DataFlow::Node { + abstract string getSinkType(); + } + + /** + * A sanitizer for SQL-injection vulnerabilities. + */ + abstract class Sanitizer extends DataFlow::Node { } + + /** A source of user input, considered as a flow source for command injection. */ + class FlowSourceAsSource extends Source instanceof SourceNode { + override string getSourceType() { result = "user-provided value" } + } + + class InvokeSqlCmdSink extends Sink { + InvokeSqlCmdSink() { + exists(DataFlow::CallNode call | call.matchesName("Invoke-Sqlcmd") | + this = call.getNamedArgument("query") + or + not call.hasNamedArgument("query") and + this = call.getArgument(0) + ) + } + + override string getSinkType() { result = "call to Invoke-Sqlcmd" } + } + + class ConnectionStringWriteSink extends Sink { + string memberName; + + ConnectionStringWriteSink() { + exists(CfgNodes::StmtNodes::AssignStmtCfgNode assign | + memberName = "CommandText" and + assign + .getLeftHandSide() + .(CfgNodes::ExprNodes::MemberExprCfgNode) + .memberNameMatches(memberName) and + assign.getRightHandSide() = this.asExpr() + ) + } + + override string getSinkType() { result = "write to " + memberName } + } + + class SqlCmdSink extends Sink { + SqlCmdSink() { + exists(DataFlow::CallOperatorNode call | + call.getCommand().asExpr().getValue().stringMatches("sqlcmd") and + call.getAnArgument() = this + ) + } + + override string getSinkType() { result = "call to sqlcmd" } + } +} diff --git a/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll new file mode 100644 index 00000000000..752b691c54a --- /dev/null +++ b/powershell/ql/lib/semmle/code/powershell/security/SqlInjectionQuery.qll @@ -0,0 +1,26 @@ +/** + * Provides a taint tracking configuration for reasoning about + * SQL-injection vulnerabilities (CWE-078). + * + * Note, for performance reasons: only import this file if + * `SqlInjectionFlow` is needed, otherwise + * `SqlInjectionCustomizations` should be imported instead. + */ + +import powershell +import semmle.code.powershell.dataflow.TaintTracking +import SqlInjectionCustomizations::SqlInjection +import semmle.code.powershell.dataflow.DataFlow + +private module Config implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof Source } + + predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } +} + +/** + * Taint-tracking for reasoning about SQL-injection vulnerabilities. + */ +module SqlInjectionFlow = TaintTracking::Global; diff --git a/powershell/ql/src/experimental/UseOfReservedCmdletChar.ql b/powershell/ql/src/experimental/UseOfReservedCmdletChar.ql index 7d610f5cdf4..3b43c5b6ce8 100644 --- a/powershell/ql/src/experimental/UseOfReservedCmdletChar.ql +++ b/powershell/ql/src/experimental/UseOfReservedCmdletChar.ql @@ -26,5 +26,5 @@ class ReservedCharacter extends string { } from Function f, ReservedCharacter r -where f.getName().matches("%"+ r + "%") +where f.getLowerCaseName().matches("%"+ r + "%") select f, "Function name contains a reserved character: " + r \ No newline at end of file diff --git a/powershell/ql/src/queries/security/cwe-089/SqlInjection.qhelp b/powershell/ql/src/queries/security/cwe-089/SqlInjection.qhelp new file mode 100644 index 00000000000..abc2fcc4acc --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-089/SqlInjection.qhelp @@ -0,0 +1,45 @@ + + + +

If a SQL query is built using string concatenation, and the +components of the concatenation include user input, a user +is likely to be able to run malicious database queries.

+
+ + +

Usually, it is better to use a prepared statement than to build a +complete query with string concatenation. A prepared statement can +include a parameter, written as either a question mark (?) or with +an explicit name (@parameter), for each part of the SQL query that is +expected to be filled in by a different value each time it is run. +When the query is later executed, a value must be +supplied for each parameter in the query.

+ +

It is good practice to use prepared statements for supplying +parameters to a query, whether or not any of the parameters are +directly traceable to user input. Doing so avoids any need to worry +about quoting and escaping.

+
+ + +

In the following example, the code runs a simple SQL query in two different ways.

+ +

The first way involves building a query, query1, by interpolating a +user-supplied text value with some string literals. The value can include special +characters, so this code allows for SQL injection attacks.

+ +

The second way builds a query, query2, with a +single string literal that includes a parameter (@username). The parameter +is then given a value by providing a hash table $params when executing the +query. This version is immune to injection attacks, because any special characters are +not given any special treatment.

+ + +
+ + +
  • MSDN: How To: Protect From SQL Injection in ASP.NET.
  • +
    +
    diff --git a/powershell/ql/src/queries/security/cwe-089/SqlInjection.ql b/powershell/ql/src/queries/security/cwe-089/SqlInjection.ql new file mode 100644 index 00000000000..86f819c3cc5 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-089/SqlInjection.ql @@ -0,0 +1,24 @@ +/** + * @name SQL query built from user-controlled sources + * @description Building a SQL query from user-controlled sources is vulnerable to insertion of + * malicious SQL code by the user. + * @kind path-problem + * @problem.severity error + * @security-severity 8.8 + * @precision high + * @id powershell/microsoft/public/sql-injection + * @tags correctness + * security + * external/cwe/cwe-089 + */ + +import powershell +import semmle.code.powershell.security.SqlInjectionQuery +import SqlInjectionFlow::PathGraph + +from SqlInjectionFlow::PathNode source, SqlInjectionFlow::PathNode sink, Source sourceNode +where + SqlInjectionFlow::flowPath(source, sink) and + sourceNode = source.getNode() +select sink.getNode(), source, sink, "This SQL query depends on a $@.", sourceNode, + sourceNode.getSourceType() diff --git a/powershell/ql/src/queries/security/cwe-089/examples/SqlInjection.ps1 b/powershell/ql/src/queries/security/cwe-089/examples/SqlInjection.ps1 new file mode 100644 index 00000000000..c449536a662 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-089/examples/SqlInjection.ps1 @@ -0,0 +1,16 @@ +param( + [string]$userinput +) + +# BAD: The user input is directly interpolated into the SQL query string +$query1 = "SELECT * FROM users WHERE name = '$userinput'" +Invoke-Sqlcmd -ServerInstance "MyServer" -Database "MyDatabase" -Query $query + +# GOOD: Using parameters to prevent SQL injection +$query2 = "SELECT * FROM users WHERE name = @username" + +$params = @{ + username = $userinput +} + +Invoke-Sqlcmd -ServerInstance "MyServer" -Database "MyDatabase" -Query $query -QueryParameters $params \ No newline at end of file diff --git a/powershell/ql/test/library-tests/ast/Expressions/expressions.expected b/powershell/ql/test/library-tests/ast/Expressions/expressions.expected index dd93724801a..cebf2918950 100644 --- a/powershell/ql/test/library-tests/ast/Expressions/expressions.expected +++ b/powershell/ql/test/library-tests/ast/Expressions/expressions.expected @@ -4,7 +4,7 @@ binaryExpr cmdExpr | BinaryExpression.ps1:4:1:4:7 | [Stmt] result | BinaryExpression.ps1:4:1:4:7 | result | | ExpandableString.ps1:1:1:1:39 | [Stmt] Date: $([DateTime]::Now)\nName: $name | ExpandableString.ps1:1:1:1:39 | Date: $([DateTime]::Now)\nName: $name | -| ExpandableString.ps1:1:23:1:37 | [Stmt] Now | ExpandableString.ps1:1:23:1:37 | Now | +| ExpandableString.ps1:1:23:1:37 | [Stmt] now | ExpandableString.ps1:1:23:1:37 | now | | MemberExpression.ps1:2:1:2:14 | [Stmt] ... | MemberExpression.ps1:2:1:2:14 | ... | | SubExpression.ps1:1:1:1:23 | [Stmt] Call to adddays | SubExpression.ps1:1:1:1:23 | Call to adddays | | SubExpression.ps1:1:3:1:10 | [Stmt] Call to get-date | SubExpression.ps1:1:3:1:10 | Call to get-date | @@ -15,5 +15,5 @@ invokeMemoryExpression expandableString | ExpandableString.ps1:1:1:1:39 | Date: $([DateTime]::Now)\nName: $name | 1 | ExpandableString.ps1:1:21:1:38 | $(...) | memberExpr -| ExpandableString.ps1:1:23:1:37 | Now | ExpandableString.ps1:1:23:1:32 | DateTime | +| ExpandableString.ps1:1:23:1:37 | now | ExpandableString.ps1:1:23:1:32 | DateTime | | MemberExpression.ps1:2:1:2:14 | ... | MemberExpression.ps1:2:1:2:10 | DateTime | diff --git a/powershell/ql/test/library-tests/ast/parent.expected b/powershell/ql/test/library-tests/ast/parent.expected index b15f8fd9037..8c4585fc5df 100644 --- a/powershell/ql/test/library-tests/ast/parent.expected +++ b/powershell/ql/test/library-tests/ast/parent.expected @@ -229,11 +229,11 @@ | Expressions/ExpandableString.ps1:1:1:1:39 | {...} | Expressions/ExpandableString.ps1:1:1:1:39 | toplevel function for ExpandableString.ps1 | | Expressions/ExpandableString.ps1:1:1:1:39 | {...} | Expressions/ExpandableString.ps1:1:1:1:39 | {...} | | Expressions/ExpandableString.ps1:1:21:1:38 | $(...) | Expressions/ExpandableString.ps1:1:1:1:39 | Date: $([DateTime]::Now)\nName: $name | -| Expressions/ExpandableString.ps1:1:23:1:32 | DateTime | Expressions/ExpandableString.ps1:1:23:1:37 | Now | -| Expressions/ExpandableString.ps1:1:23:1:37 | Now | Expressions/ExpandableString.ps1:1:23:1:37 | [Stmt] Now | -| Expressions/ExpandableString.ps1:1:23:1:37 | [Stmt] Now | Expressions/ExpandableString.ps1:1:23:1:37 | {...} | +| Expressions/ExpandableString.ps1:1:23:1:32 | DateTime | Expressions/ExpandableString.ps1:1:23:1:37 | now | +| Expressions/ExpandableString.ps1:1:23:1:37 | [Stmt] now | Expressions/ExpandableString.ps1:1:23:1:37 | {...} | +| Expressions/ExpandableString.ps1:1:23:1:37 | now | Expressions/ExpandableString.ps1:1:23:1:37 | [Stmt] now | | Expressions/ExpandableString.ps1:1:23:1:37 | {...} | Expressions/ExpandableString.ps1:1:21:1:38 | $(...) | -| Expressions/ExpandableString.ps1:1:35:1:37 | Now | Expressions/ExpandableString.ps1:1:23:1:37 | Now | +| Expressions/ExpandableString.ps1:1:35:1:37 | Now | Expressions/ExpandableString.ps1:1:23:1:37 | now | | Expressions/MemberExpression.ps1:1:1:2:14 | [synth] pipeline | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | Expressions/MemberExpression.ps1:1:1:2:14 | toplevel function for MemberExpression.ps1 | | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | diff --git a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected new file mode 100644 index 00000000000..c97f5fdd83b --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected @@ -0,0 +1,17 @@ +edges +| test.ps1:1:14:1:45 | Call to read-host | test.ps1:5:72:5:77 | query | provenance | Src:MaD:11464 | +| test.ps1:1:14:1:45 | Call to read-host | test.ps1:9:72:9:77 | query | provenance | Src:MaD:11464 | +| test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Src:MaD:11464 | +| test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Src:MaD:11464 | +nodes +| test.ps1:1:14:1:45 | Call to read-host | semmle.label | Call to read-host | +| test.ps1:5:72:5:77 | query | semmle.label | query | +| test.ps1:9:72:9:77 | query | semmle.label | query | +| test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | +| test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | +subpaths +#select +| test.ps1:5:72:5:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:5:72:5:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value | +| test.ps1:9:72:9:77 | query | test.ps1:1:14:1:45 | Call to read-host | test.ps1:9:72:9:77 | query | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value | +| test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value | +| test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:1:14:1:45 | Call to read-host | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | This SQL query depends on a $@. | test.ps1:1:14:1:45 | Call to read-host | user-provided value | diff --git a/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.qlref b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.qlref new file mode 100644 index 00000000000..302dc18db39 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-089/SqlInjection.qlref @@ -0,0 +1 @@ +queries/security/cwe-089/SqlInjection.ql \ No newline at end of file diff --git a/powershell/ql/test/query-tests/security/cwe-089/test.ps1 b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 new file mode 100644 index 00000000000..f7b8f400978 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-089/test.ps1 @@ -0,0 +1,56 @@ +$userinput = Read-Host "Please enter a value" + +# Example using Invoke-Sqlcmd with string interpolation +$query = "SELECT * FROM MyTable WHERE MyColumn = '$userinput'" +Invoke-Sqlcmd -ServerInstance "MyServer" -Database "MyDatabase" -Query $query # BAD + +# Example using Invoke-Sqlcmd with string concatenation +$query = "SELECT * FROM MyTable WHERE " + $userinput +Invoke-Sqlcmd -ServerInstance "MyServer" -Database "MyDatabase" -Query $query # BAD + +#Example using System.Data.SqlClient +$connection = New-Object System.Data.SqlClient.SqlConnection +$connection.ConnectionString = "Server=MyServer;Database=MyDatabase;" +$connection.Open() + +$command = $connection.CreateCommand() +$command.CommandText = "SELECT * FROM MyTable WHERE MyColumn = '$userinput'" # BAD +$reader = $command.ExecuteReader() +$reader.Close() +$connection.Close() + +# Example using System.Data.OleDb +$connection = New-Object System.Data.OleDb.OleDbConnection +$connection.ConnectionString = "Provider=SQLOLEDB;Data Source=MyServer;Initial Catalog=MyDatabase;" +$connection.Open() + +$command = $connection.CreateCommand() +$command.CommandText = "SELECT * FROM MyTable WHERE MyColumn = '$userinput'" # BAD +$reader = $command.ExecuteReader() +$reader.Close() +$connection.Close() + +# Example using System.Data.SqlClient with parameters +$connection = New-Object System.Data.SqlClient.SqlConnection +$connection.ConnectionString = "Server=MyServer;Database=MyDatabase;" +$connection.Open() + +$command = $connection.CreateCommand() +$command.CommandText = "SELECT * FROM MyTable WHERE MyColumn = @userinput" +$parameter = $command.Parameters.Add("@userinput", [System.Data.SqlDbType]::NVarChar) +$parameter.Value = $userinput # GOOD +$reader = $command.ExecuteReader() +$reader.Close() +$connection.Close() + +# Example using System.Data.OleDb with parameters +$connection = New-Object System.Data.OleDb.OleDbConnection +$connection.ConnectionString = "Provider=SQLOLEDB;Data Source=MyServer;Initial Catalog=MyDatabase;" +$connection.Open() +$command = $connection.CreateCommand() +$command.CommandText = "SELECT * FROM MyTable WHERE MyColumn = ?" +$parameter = $command.Parameters.Add("?", [System.Data.OleDb.OleDbType]::VarChar) +$parameter.Value = $userinput # GOOD +$reader = $command.ExecuteReader() +$reader.Close() +$connection.Close() \ No newline at end of file