hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-05 13:45:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #399 from sauyon/stored-xss

Browse Source 
Add stored XSS query
This commit is contained in:
Sauyon Lee
2020-11-19 23:23:21 -08:00
committed by GitHub
parent 93a7cc944a 7279d4090d
commit 793d6f6053
22 changed files with 784 additions and 278 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
change-notes/2020-11-11-stored-xss.md Normal file
View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* A new query "Stored cross-site scripting" (`go/stored-xss`) has been added. The query detects HTTP request responses that contain data from a database or a similar possibly user-controllable source.

15
ql/src/Security/CWE-079/StoredXss.go Normal file
View File

@@ -0,0 +1,15 @@
package main
import (
"io"
"io/ioutil"
"net/http"
)
func ListFiles(w http.ResponseWriter, r *http.Request) {
files, _ := ioutil.ReadDir(".")
for _, file := range files {
io.WriteString(w, file.Name()+"\n")
}
}

63
ql/src/Security/CWE-079/StoredXss.qhelp Normal file
View File

@@ -0,0 +1,63 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Directly using externally-controlled stored values (for example, file names or database contents) to
create HTML content without properly sanitizing the input first,
allows for a cross-site scripting vulnerability.
</p>
<p>
This kind of vulnerability is also called <i>stored</i> cross-site
scripting, to distinguish it from other types of cross-site scripting.
</p>
</overview>
<recommendation>
<p>
To guard against cross-site scripting, consider using contextual
output encoding/escaping before using uncontrolled stored values to
create HTML content, or one of the other solutions that are mentioned
in the references.
</p>
</recommendation>
<example>
<p>
The following example code writes file names directly to an HTTP
response. This leaves the website vulnerable to cross-site scripting,
if an attacker can choose the file names on the disk.
</p>
<sample src="StoredXss.go" />
<p>
Sanitizing the file names prevents the vulnerability:
</p>
<sample src="StoredXssGood.go" />
</example>
<references>
<li>
OWASP:
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html">XSS
(Cross Site Scripting) Prevention Cheat Sheet</a>.
</li>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting">Types of Cross-Site
Scripting</a>.
</li>
<li>
Wikipedia: <a href="http://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
</li>
</references>
</qhelp>

21
ql/src/Security/CWE-079/StoredXss.ql Normal file
View File

@@ -0,0 +1,21 @@
/**
* @name Stored cross-site scripting
* @description Using uncontrolled stored values in HTML allows for
* a stored cross-site scripting vulnerability.
* @kind path-problem
* @problem.severity error
* @precision low
* @id go/stored-xss
* @tags security
* external/cwe/cwe-079
* external/cwe/cwe-116
*/
import go
import semmle.go.security.StoredXss::StoredXss
import DataFlow::PathGraph
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Stored cross-site scripting vulnerability due to $@.",
source.getNode(), "stored value"

16
ql/src/Security/CWE-079/StoredXssGood.go Normal file
View File

@@ -0,0 +1,16 @@
package main
import (
"html"
"io"
"io/ioutil"
"net/http"
)
func ListFiles1(w http.ResponseWriter, r *http.Request) {
files, _ := ioutil.ReadDir(".")
for _, file := range files {
io.WriteString(w, html.EscapeString(file.Name())+"\n")
}
}

148
ql/src/semmle/go/frameworks/SQL.qll
View File

@@ -6,76 +6,50 @@ import go
/** Provides classes for working with SQL-related APIs. */
module SQL {
private class SqlFunctionModels extends TaintTracking::FunctionModel {
FunctionInput inp;
FunctionOutput outp;
/**
* A data-flow node that represents a SQL query.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `SQL::Query::Range` instead.
*/
class Query extends DataFlow::Node {
Query::Range self;
SqlFunctionModels() {
// signature: func Named(name string, value interface{}) NamedArg
hasQualifiedName("database/sql", "Named") and
(inp.isParameter(_) and outp.isResult())
}
Query() { this = self }
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input = inp and output = outp
}
/** Gets a result of this query execution. */
DataFlow::Node getAResult() { result = self.getAResult() }
/**
* Gets a query string that is used as (part of) this SQL query.
*
* Note that this may not resolve all `QueryString`s that should be associated with this
* query due to data flow.
*/
QueryString getAQueryString() { result = self.getAQueryString() }
}
private class SqlMethodModels extends TaintTracking::FunctionModel, Method {
FunctionInput inp;
FunctionOutput outp;
/**
* A data-flow node that represents a SQL query.
*/
module Query {
/**
* A data-flow node that represents a SQL query.
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `SQL::Query` instead.
*/
abstract class Range extends DataFlow::Node {
/** Gets a result of this query execution. */
abstract DataFlow::Node getAResult();
SqlMethodModels() {
// signature: func (*Row).Scan(dest ...interface{}) error
this.hasQualifiedName("database/sql", "Row", "Scan") and
(inp.isReceiver() and outp.isParameter(_))
or
// signature: func (*Rows).Scan(dest ...interface{}) error
this.hasQualifiedName("database/sql", "Rows", "Scan") and
(inp.isReceiver() and outp.isParameter(_))
or
// signature: func (Scanner).Scan(src interface{}) error
this.implements("database/sql", "Scanner", "Scan") and
(inp.isParameter(0) and outp.isReceiver())
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input = inp and output = outp
}
}
private class SqlDriverMethodModels extends TaintTracking::FunctionModel, Method {
FunctionInput inp;
FunctionOutput outp;
SqlDriverMethodModels() {
// signature: func (NotNull).ConvertValue(v interface{}) (Value, error)
this.hasQualifiedName("database/sql/driver", "NotNull", "ConvertValue") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (Null).ConvertValue(v interface{}) (Value, error)
this.hasQualifiedName("database/sql/driver", "Null", "ConvertValue") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (ValueConverter).ConvertValue(v interface{}) (Value, error)
this.implements("database/sql/driver", "ValueConverter", "ConvertValue") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (Conn).Prepare(query string) (Stmt, error)
this.implements("database/sql/driver", "Conn", "Prepare") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (ConnPrepareContext).PrepareContext(ctx context.Context, query string) (Stmt, error)
this.implements("database/sql/driver", "ConnPrepareContext", "PrepareContext") and
(inp.isParameter(1) and outp.isResult(0))
or
// signature: func (Valuer).Value() (Value, error)
this.implements("database/sql/driver", "Valuer", "Value") and
(inp.isReceiver() and outp.isResult(0))
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input = inp and output = outp
/**
* Gets a query string that is used as (part of) this SQL query.
*
* Note that this does not have to resolve all `QueryString`s that should be associated with this
* query due to data flow.
*/
abstract QueryString getAQueryString();
}
}
@@ -101,50 +75,6 @@ module SQL {
*/
abstract class Range extends DataFlow::Node { }
/** A query string used in an API function of the standard `database/sql` package. */
private class StandardQueryString extends Range {
StandardQueryString() {
exists(Method meth, string base, string m, int n |
(
meth.hasQualifiedName("database/sql", "DB", m) or
meth.hasQualifiedName("database/sql", "Tx", m) or
meth.hasQualifiedName("database/sql", "Conn", m)
) and
this = meth.getACall().getArgument(n)
|
(base = "Exec" or base = "Prepare" or base = "Query" or base = "QueryRow") and
(
m = base and n = 0
or
m = base + "Context" and n = 1
)
)
}
}
/** A query string used in an API function of the standard `database/sql/driver` package. */
private class DriverQueryString extends Range {
DriverQueryString() {
exists(Method meth, int n |
(
meth.hasQualifiedName("database/sql/driver", "Execer", "Exec") and n = 0
or
meth.hasQualifiedName("database/sql/driver", "ExecerContext", "ExecContext") and n = 1
or
meth.hasQualifiedName("database/sql/driver", "Conn", "Prepare") and n = 0
or
meth.hasQualifiedName("database/sql/driver", "ConnPrepareContext", "PrepareContext") and
n = 1
or
meth.hasQualifiedName("database/sql/driver", "Queryer", "Query") and n = 0
or
meth.hasQualifiedName("database/sql/driver", "QueryerContext", "QueryContext") and n = 1
) and
this = meth.getACall().getArgument(n)
)
}
}
/**
* An argument to an API of the squirrel library that is directly interpreted as SQL without
* taking syntactic structure into account.

1
ql/src/semmle/go/frameworks/Stdlib.qll
View File

@@ -21,6 +21,7 @@ import semmle.go.frameworks.stdlib.CryptoCipher
import semmle.go.frameworks.stdlib.CryptoRsa
import semmle.go.frameworks.stdlib.CryptoTls
import semmle.go.frameworks.stdlib.CryptoX509
import semmle.go.frameworks.stdlib.DatabaseSql
import semmle.go.frameworks.stdlib.Encoding
import semmle.go.frameworks.stdlib.EncodingAscii85
import semmle.go.frameworks.stdlib.EncodingAsn1

191
ql/src/semmle/go/frameworks/stdlib/DatabaseSql.qll Normal file
View File

@@ -0,0 +1,191 @@
/**
* Provides classes modeling security-relevant aspects of the `database/sql` package.
*/
import go
/**
* Provides classes modeling security-relevant aspects of the `database/sql` package.
*/
module DatabaseSql {
/** A query from the `database/sql` package. */
private class Query extends SQL::Query::Range, DataFlow::MethodCallNode {
string t;
Query() {
exists(Method meth, string base, string m |
meth.hasQualifiedName("database/sql", t, m) and
this = meth.getACall()
|
t = ["DB", "Tx", "Conn", "Stmt"] and
base = ["Exec", "Query", "QueryRow"] and
(m = base or m = base + "Context")
)
}
override DataFlow::Node getAResult() { result = this.getResult(0) }
override SQL::QueryString getAQueryString() {
result = this.getAnArgument()
or
// attempt to resolve a `QueryString` for `Stmt`s using local data flow.
t = "Stmt" and
result = this.getReceiver().getAPredecessor*().(DataFlow::MethodCallNode).getAnArgument()
}
}
/** A query string used in an API function of the `database/sql` package. */
private class QueryString extends SQL::QueryString::Range {
QueryString() {
exists(Method meth, string base, string t, string m, int n |
t = ["DB", "Tx", "Conn"] and
meth.hasQualifiedName("database/sql", t, m) and
this = meth.getACall().getArgument(n)
|
base = ["Exec", "Prepare", "Query", "QueryRow"] and
(
m = base and n = 0
or
m = base + "Context" and n = 1
)
)
}
}
/** A query in the standard `database/sql/driver` package. */
private class DriverQuery extends SQL::Query::Range, DataFlow::MethodCallNode {
DriverQuery() {
exists(Method meth |
(
meth.hasQualifiedName("database/sql/driver", "Execer", "Exec")
or
meth.hasQualifiedName("database/sql/driver", "ExecerContext", "ExecContext")
or
meth.hasQualifiedName("database/sql/driver", "Queryer", "Query")
or
meth.hasQualifiedName("database/sql/driver", "QueryerContext", "QueryContext")
or
meth.hasQualifiedName("database/sql/driver", "Stmt", "Exec")
or
meth.hasQualifiedName("database/sql/driver", "Stmt", "Query")
or
meth.hasQualifiedName("database/sql/driver", "StmtQueryContext", "QueryContext")
) and
this = meth.getACall()
)
}
override DataFlow::Node getAResult() { result = this.getResult(0) }
override SQL::QueryString getAQueryString() {
result = this.getAnArgument()
or
this.getTarget().hasQualifiedName("database/sql/driver", "Stmt") and
result = this.getReceiver().getAPredecessor*().(DataFlow::MethodCallNode).getAnArgument()
}
}
/** A query string used in an API function of the standard `database/sql/driver` package. */
private class DriverQueryString extends SQL::QueryString::Range {
DriverQueryString() {
exists(Method meth, int n |
(
meth.hasQualifiedName("database/sql/driver", "Execer", "Exec") and n = 0
or
meth.hasQualifiedName("database/sql/driver", "ExecerContext", "ExecContext") and n = 1
or
meth.hasQualifiedName("database/sql/driver", "Conn", "Prepare") and n = 0
or
meth.hasQualifiedName("database/sql/driver", "ConnPrepareContext", "PrepareContext") and
n = 1
or
meth.hasQualifiedName("database/sql/driver", "Queryer", "Query") and n = 0
or
meth.hasQualifiedName("database/sql/driver", "QueryerContext", "QueryContext") and n = 1
) and
this = meth.getACall().getArgument(n)
)
}
}
private class SqlFunctionModels extends TaintTracking::FunctionModel {
FunctionInput inp;
FunctionOutput outp;
SqlFunctionModels() {
// signature: func Named(name string, value interface{}) NamedArg
hasQualifiedName("database/sql", "Named") and
(inp.isParameter(_) and outp.isResult())
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input = inp and output = outp
}
}
private class SqlMethodModels extends TaintTracking::FunctionModel, Method {
FunctionInput inp;
FunctionOutput outp;
SqlMethodModels() {
// signature: func (*Row).Scan(dest ...interface{}) error
this.hasQualifiedName("database/sql", "Row", "Scan") and
(inp.isReceiver() and outp.isParameter(_))
or
// signature: func (*Rows).Scan(dest ...interface{}) error
this.hasQualifiedName("database/sql", "Rows", "Scan") and
(inp.isReceiver() and outp.isParameter(_))
or
// signature: func (Scanner).Scan(src interface{}) error
this.implements("database/sql", "Scanner", "Scan") and
(inp.isParameter(0) and outp.isReceiver())
or
// Prepare methods
this.hasQualifiedName("database/sql", ["Tx", "Db"], "Prepare") and
(inp.isParameter(0) and outp.isResult(0))
or
// PrepareContext methods
this.hasQualifiedName("database/sql", ["Tx", "Db", "Conn"], "PrepareContext") and
(inp.isParameter(1) and outp.isResult(0))
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input = inp and output = outp
}
}
private class SqlDriverMethodModels extends TaintTracking::FunctionModel, Method {
FunctionInput inp;
FunctionOutput outp;
SqlDriverMethodModels() {
// signature: func (NotNull).ConvertValue(v interface{}) (Value, error)
this.hasQualifiedName("database/sql/driver", "NotNull", "ConvertValue") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (Null).ConvertValue(v interface{}) (Value, error)
this.hasQualifiedName("database/sql/driver", "Null", "ConvertValue") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (ValueConverter).ConvertValue(v interface{}) (Value, error)
this.implements("database/sql/driver", "ValueConverter", "ConvertValue") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (Conn).Prepare(query string) (Stmt, error)
this.implements("database/sql/driver", "Conn", "Prepare") and
(inp.isParameter(0) and outp.isResult(0))
or
// signature: func (ConnPrepareContext).PrepareContext(ctx context.Context, query string) (Stmt, error)
this.implements("database/sql/driver", "ConnPrepareContext", "PrepareContext") and
(inp.isParameter(1) and outp.isResult(0))
or
// signature: func (Valuer).Value() (Value, error)
this.implements("database/sql/driver", "Valuer", "Value") and
(inp.isReceiver() and outp.isResult(0))
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input = inp and output = outp
}
}
}

107
ql/src/semmle/go/security/ReflectedXssCustomizations.qll
View File

@@ -1,8 +1,9 @@
/**
* Provides classes and predicates used by the XSS queries.
* Provides classes and predicates used by the Reflected XSS query.
*/
import go
import Xss
/**
* Provides extension points for customizing the taint-tracking configuration for reasoning about
@@ -13,15 +14,7 @@ module ReflectedXss {
abstract class Source extends DataFlow::Node { }
/** A data flow sink for reflected XSS vulnerabilities. */
abstract class Sink extends DataFlow::Node {
/**
* Gets the kind of vulnerability to report in the alert message.
*
* Defaults to `Cross-site scripting`, but may be overriden for sinks
* that do not allow script injection, but injection of other undesirable HTML elements.
*/
string getVulnerabilityKind() { result = "Cross-site scripting" }
}
abstract class Sink extends DataFlow::Node { }
/** A sanitizer for reflected XSS vulnerabilities. */
abstract class Sanitizer extends DataFlow::Node { }
@@ -31,51 +24,18 @@ module ReflectedXss {
*/
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
/**
* An expression that is sent as part of an HTTP response body, considered as an
* XSS sink.
*
* We exclude cases where the route handler sets either an unknown content type or
* a content type that does not (case-insensitively) contain the string "html". This
* is to prevent us from flagging plain-text or JSON responses as vulnerable.
*/
class HttpResponseBodySink extends Sink, HTTP::ResponseBody {
HttpResponseBodySink() { not nonHtmlContentType(this) }
/** A shared XSS sanitizer as a sanitizer for reflected XSS. */
private class SharedXssSanitizer extends Sanitizer {
SharedXssSanitizer() { this instanceof SharedXss::Sanitizer }
}
/**
* Holds if `body` specifies the response's content type to be HTML.
*/
private predicate htmlTypeSpecified(HTTP::ResponseBody body) {
body.getAContentType().regexpMatch("(?i).*html.*")
}
/** A shared XSS sanitizer guard as a sanitizer guard for reflected XSS. */
private class SharedXssSanitizerGuard extends SanitizerGuard {
SharedXss::SanitizerGuard self;
/**
* Holds if `body` may send a response with a content type other than HTML.
*/
private predicate nonHtmlContentType(HTTP::ResponseBody body) {
not htmlTypeSpecified(body) and
(
exists(body.getAContentType())
or
exists(body.getAContentTypeNode())
or
exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") |
body = call.getAnArgument() and
// checks that the format value does not start with (ignoring whitespace as defined by
// https://mimesniff.spec.whatwg.org/#whitespace-byte):
// - '<', which could lead to an HTML content type being detected, or
// - '%', which could be a format string.
call.getArgument(1).getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<%].*")
)
or
exists(DataFlow::Node pred | body = pred.getASuccessor*() |
// data starting with a character other than `<` (ignoring whitespace as defined by
// https://mimesniff.spec.whatwg.org/#whitespace-byte) cannot cause an HTML content type to
// be detected.
pred.getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<].*")
)
)
SharedXssSanitizerGuard() { this = self }
override predicate checks(Expr e, boolean b) { self.checks(e, b) }
}
/**
@@ -83,45 +43,8 @@ module ReflectedXss {
*/
class UntrustedFlowAsSource extends Source, UntrustedFlowSource { }
/**
* A regexp replacement involving an HTML meta-character, or a call to an escape
* function, viewed as a sanitizer for XSS vulnerabilities.
*
* The XSS queries do not attempt to reason about correctness or completeness of sanitizers,
* so any such call stops taint propagation.
*/
class MetacharEscapeSanitizer extends Sanitizer, DataFlow::CallNode {
MetacharEscapeSanitizer() {
exists(Function f | f = this.getCall().getTarget() |
f.(RegexpReplaceFunction).getRegexp(this).getPattern().regexpMatch(".*['\"&<>].*")
or
f instanceof HtmlEscapeFunction
or
f instanceof JsEscapeFunction
)
}
}
/**
* A check against a constant value, considered a barrier for reflected XSS.
*/
class EqualityTestGuard extends SanitizerGuard, DataFlow::EqualityTestNode {
override predicate checks(Expr e, boolean outcome) {
this.getAnOperand().isConst() and
e = this.getAnOperand().asExpr() and
outcome = this.getPolarity()
}
}
/**
* A JSON marshaler, acting to sanitize a possible XSS vulnerability because the
* marshaled value is very unlikely to be returned as an HTML content-type.
*/
class JsonMarshalSanitizer extends Sanitizer {
JsonMarshalSanitizer() {
exists(MarshalingFunction mf | mf.getFormat() = "JSON" |
this = mf.getOutput().getNode(mf.getACall())
)
}
/** An arbitrary XSS sink, considered as a flow sink for stored XSS. */
private class AnySink extends Sink {
AnySink() { this instanceof SharedXss::Sink }
}
}

38
ql/src/semmle/go/security/StoredXss.qll Normal file
View File

@@ -0,0 +1,38 @@
/**
* Provides a taint-tracking configuration for reasoning about stored
* cross-site scripting vulnerabilities.
*
* Note, for performance reasons: only import this file if
* `StoredXss::Configuration` is needed, otherwise
* `StoredXssCustomizations` should be imported instead.
*/
import go
/**
* Provides a taint-tracking configuration for reasoning about stored
* cross-site scripting vulnerabilities.
*/
module StoredXss {
import StoredXssCustomizations::StoredXss
/**
* A taint-tracking configuration for reasoning about XSS.
*/
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "StoredXss" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSanitizer(DataFlow::Node node) {
super.isSanitizer(node) or
node instanceof Sanitizer
}
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof SanitizerGuard
}
}
}

64
ql/src/semmle/go/security/StoredXssCustomizations.qll Normal file
View File

@@ -0,0 +1,64 @@
/**
* Provides classes and predicates used by the Stored XSS query.
*/
import go
import Xss
/** Provides classes and predicates used by the stored XSS query. */
module StoredXss {
/** A data flow source for stored XSS vulnerabilities. */
abstract class Source extends DataFlow::Node { }
/** A data flow sink for stored XSS vulnerabilities. */
abstract class Sink extends DataFlow::Node { }
/** A sanitizer for stored XSS vulnerabilities. */
abstract class Sanitizer extends DataFlow::Node { }
/** A sanitizer guard for stored XSS vulnerabilities. */
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
/** A shared XSS sanitizer as a sanitizer for stored XSS. */
private class SharedXssSanitizer extends Sanitizer {
SharedXssSanitizer() { this instanceof SharedXss::Sanitizer }
}
/** A shared XSS sanitizer guard as a sanitizer guard for stored XSS. */
private class SharedXssSanitizerGuard extends SanitizerGuard {
SharedXss::SanitizerGuard self;
SharedXssSanitizerGuard() { this = self }
override predicate checks(Expr e, boolean b) { self.checks(e, b) }
}
/** A database query result, considered as a flow source for stored XSS. */
private class DatabaseQueryAsSource extends Source {
DatabaseQueryAsSource() { this = any(SQL::Query q).getAResult() }
}
/** A file name, considered as a source for a stored XSS attack. */
class FileNameSource extends Source {
FileNameSource() {
// the second parameter to a filepath.Walk function
exists(DataFlow::ParameterNode prm, DataFlow::FunctionNode f, DataFlow::CallNode walkCall |
prm = this and
f.getParameter(0) = prm
|
walkCall.getTarget().hasQualifiedName("path/filepath", "Walk") and
walkCall.getArgument(1) = f.getASuccessor*()
)
or
// A call to os.FileInfo.Name
exists(Method m | m.implements("os", "FileInfo", "Name") |
m = this.(DataFlow::CallNode).getTarget()
)
}
}
/** An arbitrary XSS sink, considered as a flow sink for stored XSS. */
private class AnySink extends Sink {
AnySink() { this instanceof SharedXss::Sink }
}
}

117
ql/src/semmle/go/security/Xss.qll Normal file
View File

@@ -0,0 +1,117 @@
/**
* Provides classes and predicates used by the XSS queries.
*/
import go
/** Provides classes and predicates shared between the XSS queries. */
module SharedXss {
/** A data flow source for XSS vulnerabilities. */
abstract class Source extends DataFlow::Node { }
/** A data flow sink for XSS vulnerabilities. */
abstract class Sink extends DataFlow::Node {
/**
* Gets the kind of vulnerability to report in the alert message.
*
* Defaults to `Cross-site scripting`, but may be overriden for sinks
* that do not allow script injection, but injection of other undesirable HTML elements.
*/
string getVulnerabilityKind() { result = "Cross-site scripting" }
}
/** A sanitizer for XSS vulnerabilities. */
abstract class Sanitizer extends DataFlow::Node { }
/** A sanitizer guard for XSS vulnerabilities. */
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
/**
* An expression that is sent as part of an HTTP response body, considered as an
* XSS sink.
*
* We exclude cases where the route handler sets either an unknown content type or
* a content type that does not (case-insensitively) contain the string "html". This
* is to prevent us from flagging plain-text or JSON responses as vulnerable.
*/
class HttpResponseBodySink extends Sink, HTTP::ResponseBody {
HttpResponseBodySink() { not nonHtmlContentType(this) }
}
/**
* Holds if `body` may send a response with a content type other than HTML.
*/
private predicate nonHtmlContentType(HTTP::ResponseBody body) {
not htmlTypeSpecified(body) and
(
exists(body.getAContentType())
or
exists(body.getAContentTypeNode())
or
exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") |
body = call.getAnArgument() and
// checks that the format value does not start with (ignoring whitespace as defined by
// https://mimesniff.spec.whatwg.org/#whitespace-byte):
// - '<', which could lead to an HTML content type being detected, or
// - '%', which could be a format string.
call.getArgument(1).getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<%].*")
)
or
exists(DataFlow::Node pred | body = pred.getASuccessor*() |
// data starting with a character other than `<` (ignoring whitespace as defined by
// https://mimesniff.spec.whatwg.org/#whitespace-byte) cannot cause an HTML content type to
// be detected.
pred.getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<].*")
)
)
}
/**
* Holds if `body` specifies the response's content type to be HTML.
*/
private predicate htmlTypeSpecified(HTTP::ResponseBody body) {
body.getAContentType().regexpMatch("(?i).*html.*")
}
/**
* A JSON marshaler, acting to sanitize a possible XSS vulnerability because the
* marshaled value is very unlikely to be returned as an HTML content-type.
*/
class JsonMarshalSanitizer extends Sanitizer {
JsonMarshalSanitizer() {
exists(MarshalingFunction mf | mf.getFormat() = "JSON" |
this = mf.getOutput().getNode(mf.getACall())
)
}
}
/**
* A regexp replacement involving an HTML meta-character, or a call to an escape
* function, viewed as a sanitizer for XSS vulnerabilities.
*
* The XSS queries do not attempt to reason about correctness or completeness of sanitizers,
* so any such call stops taint propagation.
*/
class MetacharEscapeSanitizer extends Sanitizer, DataFlow::CallNode {
MetacharEscapeSanitizer() {
exists(Function f | f = this.getCall().getTarget() |
f.(RegexpReplaceFunction).getRegexp(this).getPattern().regexpMatch(".*['\"&<>].*")
or
f instanceof HtmlEscapeFunction
or
f instanceof JsEscapeFunction
)
}
}
/**
* A check against a constant value, considered a barrier for XSS.
*/
class EqualityTestGuard extends SanitizerGuard, DataFlow::EqualityTestNode {
override predicate checks(Expr e, boolean outcome) {
this.getAnOperand().isConst() and
e = this.getAnOperand().asExpr() and
outcome = this.getPolarity()
}
}
}

37
ql/test/library-tests/semmle/go/frameworks/SQL/QueryString.expected
View File

@@ -1,37 +0,0 @@
| main.go:13:10:13:14 | query |
| main.go:14:22:14:26 | query |
| main.go:15:13:15:17 | query |
| main.go:16:25:16:29 | query |
| main.go:17:11:17:15 | query |
| main.go:18:23:18:27 | query |
| main.go:19:14:19:18 | query |
| main.go:20:26:20:30 | query |
| main.go:24:57:24:65 | querypart |
| main.go:25:44:25:52 | querypart |
| main.go:29:10:29:14 | query |
| main.go:30:22:30:26 | query |
| main.go:31:13:31:17 | query |
| main.go:32:25:32:29 | query |
| main.go:33:11:33:15 | query |
| main.go:34:23:34:27 | query |
| main.go:35:14:35:18 | query |
| main.go:36:26:36:30 | query |
| pg.go:14:7:14:11 | query |
| pg.go:16:24:16:28 | query |
| pg.go:17:15:17:19 | query |
| pg.go:18:22:18:26 | query |
| pg.go:19:13:19:17 | query |
| pg.go:20:22:20:26 | query |
| pg.go:21:13:21:17 | query |
| pg.go:26:10:26:14 | query |
| pg.go:27:15:27:19 | query |
| pg.go:28:13:28:17 | query |
| pg.go:29:13:29:17 | query |
| pg.go:32:8:32:12 | query |
| pg.go:33:15:33:19 | query |
| pg.go:34:8:34:12 | query |
| pg.go:36:19:36:23 | query |
| pg.go:37:11:37:15 | query |
| pg.go:38:10:38:14 | query |
| pg.go:39:17:39:21 | query |
| pg.go:40:12:40:16 | query |

33
ql/test/library-tests/semmle/go/frameworks/SQL/QueryString.ql
View File

@@ -1,4 +1,33 @@
import go
import TestUtilities.InlineExpectationsTest
from SQL::QueryString qs
select qs
class SQLTest extends InlineExpectationsTest {
SQLTest() { this = "SQLTest" }
override string getARelevantTag() { result = "query" }
override predicate hasActualResult(string file, int line, string element, string tag, string value) {
tag = "query" and
exists(SQL::Query q, SQL::QueryString qs, string qsFile, int qsLine | qs = q.getAQueryString() |
q.hasLocationInfo(file, line, _, _, _) and
qs.hasLocationInfo(qsFile, qsLine, _, _, _) and
element = q.toString() and
value = qs.toString()
)
}
}
class QueryString extends InlineExpectationsTest {
QueryString() { this = "QueryString no Query" }
override string getARelevantTag() { result = "querystring" }
override predicate hasActualResult(string file, int line, string element, string tag, string value) {
tag = "querystring" and
element = "" and
exists(SQL::QueryString qs | not exists(SQL::Query q | qs = q.getAQueryString()) |
qs.hasLocationInfo(file, line, _, _, _) and
value = qs.toString()
)
}
}

73
ql/test/library-tests/semmle/go/frameworks/SQL/main.go
View File

@@ -9,31 +9,66 @@ import (
"github.com/Masterminds/squirrel"
)
func test(db *sql.DB, query string, ctx context.Context) {
db.Exec(query)
db.ExecContext(ctx, query)
db.Prepare(query)
db.PrepareContext(ctx, query)
db.Query(query)
db.QueryContext(ctx, query)
db.QueryRow(query)
db.QueryRowContext(ctx, query)
var (
query1 string
query2 string
query3 string
query4 string
query5 string
query6 string
query7 string
query8 string
query11 string
query12 string
query13 string
query14 string
query15 string
query16 string
query17 string
query18 string
query21 string
query22 string
query23 string
)
func test(db *sql.DB, ctx context.Context) {
db.Exec(query1) // $query=query1
db.ExecContext(ctx, query2) // $query=query2
db.Prepare(query3) // $querystring=query3
db.PrepareContext(ctx, query4) // $querystring=query4
db.Query(query5) // $query=query5
db.QueryContext(ctx, query6) // $query=query6
db.QueryRow(query7) // $query=query7
db.QueryRowContext(ctx, query8) // $query=query8
}
func squirrelTest(querypart string) {
squirrel.Select("*").From("users").Where(squirrel.Expr(querypart))
squirrel.Select("*").From("users").Suffix(querypart)
squirrel.Select("*").From("users").Where(squirrel.Expr(querypart)) // $querystring=querypart
squirrel.Select("*").From("users").Suffix(querypart) // $querystring=querypart
}
func test2(tx *sql.Tx, query string, ctx context.Context) {
tx.Exec(query)
tx.ExecContext(ctx, query)
tx.Prepare(query)
tx.PrepareContext(ctx, query)
tx.Query(query)
tx.QueryContext(ctx, query)
tx.QueryRow(query)
tx.QueryRowContext(ctx, query)
tx.Exec(query11) // $query=query11
tx.ExecContext(ctx, query12) // $query=query12
tx.Prepare(query13) // $querystring=query13
tx.PrepareContext(ctx, query14) // $querystring=query14
tx.Query(query15) // $query=query15
tx.QueryContext(ctx, query16) // $query=query16
tx.QueryRow(query17) // $query=query17
tx.QueryRowContext(ctx, query18) // $query=query18
}
func test3(db *sql.DB, ctx context.Context) {
stmt1, _ := db.Prepare(query21) // $f+:querystring=query21
stmt1.Exec() // $f-:query=query21
stmt2, _ := db.PrepareContext(ctx, query22) // $f+:querystring=query22
stmt2.ExecContext(ctx) // $f-:query=query22
stmt3, _ := db.Prepare(query23) // $f+:querystring=query23
runQuery(stmt3)
}
func runQuery(stmt *sql.Stmt) {
stmt.Exec() // $f-:query=query23
}
func main() {}

38
ql/test/library-tests/semmle/go/frameworks/SQL/pg.go
View File

@@ -11,31 +11,31 @@ import (
)
func pgtest(query string, conn pg.Conn, db pg.DB, tx pg.Tx) {
pg.Q(query)
pg.Q(query) // $querystring=query
var dst []byte
conn.FormatQuery(dst, query)
conn.Prepare(query)
db.FormatQuery(dst, query)
db.Prepare(query)
tx.FormatQuery(dst, query)
tx.Prepare(query)
conn.FormatQuery(dst, query) // $querystring=query
conn.Prepare(query) // $querystring=query
db.FormatQuery(dst, query) // $querystring=query
db.Prepare(query) // $querystring=query
tx.FormatQuery(dst, query) // $querystring=query
tx.Prepare(query) // $querystring=query
}
// go-pg v9 dropped support for `FormatQuery`
func newpgtest(query string, conn newpg.Conn, db newpg.DB, tx newpg.Tx) {
newpg.Q(query)
conn.Prepare(query)
db.Prepare(query)
tx.Prepare(query)
newpg.Q(query) // $querystring=query
conn.Prepare(query) // $querystring=query
db.Prepare(query) // $querystring=query
tx.Prepare(query) // $querystring=query
}
func pgormtest(query string, q orm.Query) {
orm.Q(query)
q.ColumnExpr(query)
q.For(query)
orm.Q(query) // $querystring=query
q.ColumnExpr(query) // $querystring=query
q.For(query) // $querystring=query
var b []byte
q.FormatQuery(b, query)
q.Having(query)
q.Where(query)
q.WhereInMulti(query)
q.WhereOr(query)
q.FormatQuery(b, query) // $querystring=query
q.Having(query) // $querystring=query
q.Where(query) // $querystring=query
q.WhereInMulti(query) // $querystring=query
q.WhereOr(query) // $querystring=query
}

11
ql/test/query-tests/Security/CWE-079/StoredXss.expected Normal file
View File

@@ -0,0 +1,11 @@
edges
| StoredXss.go:13:21:13:31 | call to Name : string | StoredXss.go:13:21:13:36 | ...+... |
| stored.go:16:3:16:28 | ... := ...[0] : pointer type | stored.go:28:22:28:25 | name |
nodes
| StoredXss.go:13:21:13:31 | call to Name : string | semmle.label | call to Name : string |
| StoredXss.go:13:21:13:36 | ...+... | semmle.label | ...+... |
| stored.go:16:3:16:28 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
| stored.go:28:22:28:25 | name | semmle.label | name |
#select
| StoredXss.go:13:21:13:36 | ...+... | StoredXss.go:13:21:13:31 | call to Name : string | StoredXss.go:13:21:13:36 | ...+... | Stored cross-site scripting vulnerability due to $@. | StoredXss.go:13:21:13:31 | call to Name | stored value |
| stored.go:28:22:28:25 | name | stored.go:16:3:16:28 | ... := ...[0] : pointer type | stored.go:28:22:28:25 | name | Stored cross-site scripting vulnerability due to $@. | stored.go:16:3:16:28 | ... := ...[0] | stored value |

15
ql/test/query-tests/Security/CWE-079/StoredXss.go Normal file
View File

@@ -0,0 +1,15 @@
package main
import (
"io"
"io/ioutil"
"net/http"
)
func ListFiles(w http.ResponseWriter, r *http.Request) {
files, _ := ioutil.ReadDir(".")
for _, file := range files {
io.WriteString(w, file.Name()+"\n")
}
}

1
ql/test/query-tests/Security/CWE-079/StoredXss.qlref Normal file
View File

@@ -0,0 +1 @@
Security/CWE-079/StoredXss.ql

16
ql/test/query-tests/Security/CWE-079/StoredXssGood.go Normal file
View File

@@ -0,0 +1,16 @@
package main
import (
"html"
"io"
"io/ioutil"
"net/http"
)
func ListFiles1(w http.ResponseWriter, r *http.Request) {
files, _ := ioutil.ReadDir(".")
for _, file := range files {
io.WriteString(w, html.EscapeString(file.Name())+"\n")
}
}

53
ql/test/query-tests/Security/CWE-079/stored.go Normal file
View File

@@ -0,0 +1,53 @@
package main
import (
"database/sql"
"io"
"log"
"net/http"
)
var db *sql.DB
var q string
func storedserve1() {
http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
r.ParseForm()
rows, _ := db.Query(q, 32)
for rows.Next() {
var (
id int64
name string
)
if err := rows.Scan(&id, &name); err != nil {
log.Fatal(err)
}
// BAD: the stored XSS query assumes all query results are untrusted
io.WriteString(w, name)
}
})
}
func storedserve2() {
http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) {
r.ParseForm()
rows, _ := db.Query(q, 32)
for rows.Next() {
var (
id int64
name string
)
if err := rows.Scan(&id, &name); err != nil {
log.Fatal(err)
}
// GOOD: name is checked against a constant value
if name == "Sam" {
io.WriteString(w, name)
}
}
})
}

2
ql/test/query-tests/Security/CWE-079/tst.go
View File

@@ -69,3 +69,5 @@ func serve9(log io.Writer) {
})
http.ListenAndServe(":80", nil)
}
func main() {}