mirror of
https://github.com/github/codeql.git
synced 2026-01-29 22:32:58 +01:00
Merge pull request #66 from intrigus-lgtm/CWE-643
CWE-643 XPathInjection on Go
This commit is contained in:
Max Schaefer
32
ql/src/experimental/Security/CWE-643/XPathInjection.go
Normal file
32
ql/src/experimental/Security/CWE-643/XPathInjection.go
Normal file
@@ -0,0 +1,32 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/ChrisTrenkamp/goxpath"
|
||||
"github.com/ChrisTrenkamp/goxpath/tree"
|
||||
)
|
||||
|
||||
func main() {}
|
||||
|
||||
func processRequest(r *http.Request, doc tree.Node) {
|
||||
r.ParseForm()
|
||||
username := r.Form.Get("username")
|
||||
password := r.Form.Get("password")
|
||||
|
||||
// BAD: User input used directly in an XPath expression
|
||||
xPath := goxpath.MustParse("//users/user[login/text()='" + username + "' and password/text() = '" + password + "']/home_dir/text()")
|
||||
unsafeRes, _ := xPath.ExecBool(doc)
|
||||
fmt.Println(unsafeRes)
|
||||
|
||||
// GOOD: Value of parameters is defined here instead of directly in the query
|
||||
opt := func(o *goxpath.Opts) {
|
||||
o.Vars["username"] = tree.String(username)
|
||||
o.Vars["password"] = tree.String(password)
|
||||
}
|
||||
// GOOD: Uses parameters to avoid including user input directly in XPath expression
|
||||
xPath = goxpath.MustParse("//users/user[login/text()=$username and password/text() = $password]/home_dir/text()")
|
||||
safeRes, _ := xPath.ExecBool(doc, opt)
|
||||
fmt.Println(safeRes)
|
||||
}
|
||||
44
ql/src/experimental/Security/CWE-643/XPathInjection.qhelp
Normal file
44
ql/src/experimental/Security/CWE-643/XPathInjection.qhelp
Normal file
@@ -0,0 +1,44 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>
|
||||
If an XPath expression is built using string concatenation, and the components of the concatenation
|
||||
include user input, a user is likely to be able to create a malicious XPath expression.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
If user input must be included in an XPath expression, pre-compile the query and use variable
|
||||
references to include the user input.
|
||||
</p>
|
||||
<p>
|
||||
For example, when using the <code>github.com/ChrisTrenkamp/goxpath</code> API, this can be done by creating a function that takes an <code>*goxpath.Opts</code> structure.
|
||||
In this structure you can then set the values of the variable references.
|
||||
This function can then be specified when calling <code>Exec()</code>, <code>Exec{Bool|Num|Node}()</code>, <code>ParseExec()</code> or <code>MustExec()</code>.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
In the first example, the code accepts a user name specified by the user, and uses this
|
||||
unvalidated and unsanitized value in an XPath expression. This is vulnerable to the user providing
|
||||
special characters or string sequences that change the meaning of the XPath expression to search
|
||||
for different values.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
In the second example, the XPath expression is a hard-coded string that specifies some variables,
|
||||
which are safely resolved at runtime using the <code>goxpath.Opts</code> structure.
|
||||
</p>
|
||||
<sample src="XPathInjection.go" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>OWASP: <a href="https://www.owasp.org/index.php?title=Testing_for_XPath_Injection_(OTG-INPVAL-010)">Testing for XPath Injection</a>.</li>
|
||||
<li>OWASP: <a href="https://www.owasp.org/index.php/XPATH_Injection">XPath Injection</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
188
ql/src/experimental/Security/CWE-643/XPathInjection.ql
Normal file
188
ql/src/experimental/Security/CWE-643/XPathInjection.ql
Normal file
@@ -0,0 +1,188 @@
|
||||
/**
|
||||
* @name XPath injection
|
||||
* @description Building an XPath expression from user-controlled sources is vulnerable to insertion of
|
||||
* malicious code by the user.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @id go/xml/xpath-injection
|
||||
* @tags security
|
||||
* external/cwe/cwe-643
|
||||
*/
|
||||
|
||||
import go
|
||||
import DataFlow::PathGraph
|
||||
|
||||
class ByteSliceType extends SliceType {
|
||||
ByteSliceType() { this.getElementType() instanceof Uint8Type }
|
||||
}
|
||||
|
||||
class XPathInjectionConfiguration extends TaintTracking::Configuration {
|
||||
XPathInjectionConfiguration() { this = "XPathInjectionConfiguration" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof XPathInjectionSink }
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
exists(Type t | t = node.getType().getUnderlyingType() |
|
||||
not t instanceof StringType or not t instanceof ByteSliceType
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
abstract class XPathInjectionSink extends DataFlow::Node { }
|
||||
|
||||
// https://github.com/antchfx/xpath
|
||||
class XPathSink extends XPathInjectionSink {
|
||||
XPathSink() {
|
||||
exists(Function f, string name | name.matches("Compile%") |
|
||||
f.hasQualifiedName("github.com/antchfx/xpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("MustCompile%") |
|
||||
f.hasQualifiedName("github.com/antchfx/xpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("Select%") |
|
||||
f.hasQualifiedName("github.com/antchfx/xpath", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/antchfx/htmlquery
|
||||
class HtmlQuerySink extends XPathInjectionSink {
|
||||
HtmlQuerySink() {
|
||||
exists(Function f, string name | name.matches("Find%") |
|
||||
f.hasQualifiedName("github.com/antchfx/htmlquery", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("Query%") |
|
||||
f.hasQualifiedName("github.com/antchfx/htmlquery", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
or
|
||||
exists(Function f |
|
||||
f.hasQualifiedName("github.com/antchfx/htmlquery", "getQuery") and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/antchfx/xmlquery
|
||||
class XmlQuerySink extends XPathInjectionSink {
|
||||
XmlQuerySink() {
|
||||
exists(Function f, string name | name.matches("Find%") |
|
||||
f.hasQualifiedName("github.com/antchfx/xmlquery", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("Query%") |
|
||||
f.hasQualifiedName("github.com/antchfx/xmlquery", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("Select%") |
|
||||
f.hasQualifiedName("github.com/antchfx/xmlquery", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f |
|
||||
f.hasQualifiedName("github.com/antchfx/xmlquery", "getQuery") and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/antchfx/jsonquery
|
||||
class JsonQuerySink extends XPathInjectionSink {
|
||||
JsonQuerySink() {
|
||||
exists(Function f, string name | name.matches("Find%") |
|
||||
f.hasQualifiedName("github.com/antchfx/jsonquery", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("Query%") |
|
||||
f.hasQualifiedName("github.com/antchfx/jsonquery", name) and
|
||||
this = f.getACall().getArgument(1)
|
||||
)
|
||||
or
|
||||
exists(Function f |
|
||||
f.hasQualifiedName("github.com/antchfx/jsonquery", "getQuery") and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/go-xmlpath/xmlpath
|
||||
class XmlPathSink extends XPathInjectionSink {
|
||||
XmlPathSink() {
|
||||
exists(Function f, string name | name.matches("Compile%") |
|
||||
f.hasQualifiedName("github.com/go-xmlpath/xmlpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("MustCompile%") |
|
||||
f.hasQualifiedName("github.com/go-xmlpath/xmlpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/ChrisTrenkamp/goxpath
|
||||
class GoXPathSink extends XPathInjectionSink {
|
||||
GoXPathSink() {
|
||||
exists(Function f, string name | name.matches("Parse%") |
|
||||
f.hasQualifiedName("github.com/ChrisTrenkamp/goxpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("MustParse%") |
|
||||
f.hasQualifiedName("github.com/ChrisTrenkamp/goxpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/santhosh-tekuri/xpathparser
|
||||
class XPathParserSink extends XPathInjectionSink {
|
||||
XPathParserSink() {
|
||||
exists(Function f, string name | name.matches("Parse%") |
|
||||
f.hasQualifiedName("github.com/santhosh-tekuri/xpathparser", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("MustParse%") |
|
||||
f.hasQualifiedName("github.com/santhosh-tekuri/xpathparser", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/moovweb/gokogiri
|
||||
class GokogiriSink extends XPathInjectionSink {
|
||||
GokogiriSink() {
|
||||
exists(Function f, string name | name.matches("Compile%") |
|
||||
f.hasQualifiedName("github.com/moovweb/gokogiri/xpath", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("Search%") |
|
||||
f.hasQualifiedName("github.com/moovweb/gokogiri/xml", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
or
|
||||
exists(Function f, string name | name.matches("EvalXPath%") |
|
||||
f.hasQualifiedName("github.com/moovweb/gokogiri/xml", name) and
|
||||
this = f.getACall().getArgument(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
from DataFlow::PathNode source, DataFlow::PathNode sink, XPathInjectionConfiguration c
|
||||
where c.hasFlowPath(source, sink)
|
||||
select sink.getNode(), source, sink, "$@ flows here and is used in an XPath expression.",
|
||||
source.getNode(), "A user-provided value"
|
||||
Reference in New Issue
Block a user