diff --git a/ql/src/experimental/Security/CWE-643/XPathInjection.go b/ql/src/experimental/Security/CWE-643/XPathInjection.go new file mode 100644 index 00000000000..869c98acb89 --- /dev/null +++ b/ql/src/experimental/Security/CWE-643/XPathInjection.go @@ -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) +} diff --git a/ql/src/experimental/Security/CWE-643/XPathInjection.qhelp b/ql/src/experimental/Security/CWE-643/XPathInjection.qhelp new file mode 100644 index 00000000000..23d693ac704 --- /dev/null +++ b/ql/src/experimental/Security/CWE-643/XPathInjection.qhelp @@ -0,0 +1,44 @@ + + + +

+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. +

+
+ + +

+If user input must be included in an XPath expression, pre-compile the query and use variable +references to include the user input. +

+

+For example, when using the github.com/ChrisTrenkamp/goxpath API, this can be done by creating a function that takes an *goxpath.Opts structure. +In this structure you can then set the values of the variable references. +This function can then be specified when calling Exec(), Exec{Bool|Num|Node}(), ParseExec() or MustExec(). +

+ +
+ + +

+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. +

+ +

+In the second example, the XPath expression is a hard-coded string that specifies some variables, +which are safely resolved at runtime using the goxpath.Opts structure. +

+ +
+ + +
  • OWASP: Testing for XPath Injection.
  • +
  • OWASP: XPath Injection.
  • +
    +
    diff --git a/ql/src/experimental/Security/CWE-643/XPathInjection.ql b/ql/src/experimental/Security/CWE-643/XPathInjection.ql new file mode 100644 index 00000000000..453710d71e0 --- /dev/null +++ b/ql/src/experimental/Security/CWE-643/XPathInjection.ql @@ -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"