Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit bc59fa4

Browse files
author
Sauyon Lee
authored
Merge pull request #73 from intrigus-lgtm/make-CWE-643-supported
Make cwe 643 supported
2 parents eba8dd0 + be21d49 commit bc59fa4

File tree

49 files changed

+1093
-195
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1093
-195
lines changed

‎change-notes/1.24/analysis-go.md

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ The CodeQL library for Go now contains a folder of simple "cookbook" queries tha
1919
| Incomplete URL scheme check (`go/incomplete-url-scheme-check`) | correctness, security, external/cwe/cwe-020 | Highlights checks for `javascript` URLs that do not take `data` or `vbscript` URLs into account. Results are shown on LGTM by default. |
2020
| Potentially unsafe quoting (`go/unsafe-quoting`) | correctness, security, external/cwe/cwe-078, external/cwe/cwe-089, external/cwe/cwe-094 | Highlights code that constructs a quoted string literal containing data that may itself contain quotes. Results are shown on LGTM by default. |
2121
| Size computation for allocation may overflow (`go/allocation-size-overflow`) | correctness, security, external/cwe/cwe-190 | Highlights code that computes the size of an allocation based on the size of a potentially large object. Results are shown on LGTM by default. |
22+
| XPath injection (`go/xml/xpath-injection`) | security, external/cwe/cwe-643 | Highlights code that uses remote input in an XPath expression. Results are shown on LGTM by default. |
2223

2324
## Changes to existing queries
2425

‎ql/src/experimental/Security/CWE-643/XPathInjection.go renamed to ‎ql/src/Security/CWE-643/XPathInjection.go

+2-4
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,18 @@ func main() {}
1313
func processRequest(r *http.Request, doc tree.Node) {
1414
r.ParseForm()
1515
username := r.Form.Get("username")
16-
password := r.Form.Get("password")
1716

1817
// BAD: User input used directly in an XPath expression
19-
xPath := goxpath.MustParse("//users/user[login/text()='" + username + "' and password/text() = '" + password + "']/home_dir/text()")
18+
xPath := goxpath.MustParse("//users/user[login/text()='" + username + "']/home_dir/text()")
2019
unsafeRes, _ := xPath.ExecBool(doc)
2120
fmt.Println(unsafeRes)
2221

2322
// GOOD: Value of parameters is defined here instead of directly in the query
2423
opt := func(o *goxpath.Opts) {
2524
o.Vars["username"] = tree.String(username)
26-
o.Vars["password"] = tree.String(password)
2725
}
2826
// GOOD: Uses parameters to avoid including user input directly in XPath expression
29-
xPath = goxpath.MustParse("//users/user[login/text()=$username and password/text() = $password]/home_dir/text()")
27+
xPath = goxpath.MustParse("//users/user[login/text()=$username]/home_dir/text()")
3028
safeRes, _ := xPath.ExecBool(doc, opt)
3129
fmt.Println(safeRes)
3230
}

‎ql/src/experimental/Security/CWE-643/XPathInjection.qhelp renamed to ‎ql/src/Security/CWE-643/XPathInjection.qhelp

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ If user input must be included in an XPath expression, pre-compile the query and
1515
references to include the user input.
1616
</p>
1717
<p>
18-
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.
18+
For example, when using the <code>github.com/ChrisTrenkamp/goxpath</code> API, you can do this by creating a function that takes an <code>*goxpath.Opts</code> structure.
1919
In this structure you can then set the values of the variable references.
20-
This function can then be specified when calling <code>Exec()</code>, <code>Exec{Bool|Num|Node}()</code>, <code>ParseExec()</code> or <code>MustExec()</code>.
20+
This function can then be specified when calling <code>Exec()</code>, <code>Exec{Bool|Num|Node}()</code>, <code>ParseExec()</code>, or <code>MustExec()</code>.
2121
</p>
2222

2323
</recommendation>
2424

2525
<example>
2626
<p>
27-
In the first example, the code accepts a user name specified by the user, and uses this
27+
In the first example, the code accepts a username specified by the user, and uses this
2828
unvalidated and unsanitized value in an XPath expression. This is vulnerable to the user providing
2929
special characters or string sequences that change the meaning of the XPath expression to search
3030
for different values.

‎ql/src/Security/CWE-643/XPathInjection.ql

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* @name XPath injection
3+
* @description Building an XPath expression from user-controlled sources is vulnerable to insertion of
4+
* malicious code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id go/xml/xpath-injection
9+
* @tags security
10+
* external/cwe/cwe-643
11+
*/
12+
13+
import go
14+
import semmle.go.security.XPathInjection::XPathInjection
15+
import DataFlow::PathGraph
16+
17+
/** Holds if `node` is either a string or a byte slice */
18+
predicate isStringOrByte(DataFlow::PathNode node) {
19+
exists(Type t | t = node.getNode().getType().getUnderlyingType() |
20+
t instanceof StringType or t instanceof ByteSliceType
21+
)
22+
}
23+
24+
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
25+
where config.hasFlowPath(source, sink) and isStringOrByte(sink)
26+
select sink.getNode(), source, sink, "$@ flows here and is used in an XPath expression.",
27+
source.getNode(), "A user-provided value"

‎ql/src/experimental/Security/CWE-643/XPathInjection.ql

-188
This file was deleted.

‎ql/src/go.qll

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import semmle.go.dataflow.SSA
2626
import semmle.go.frameworks.HTTP
2727
import semmle.go.frameworks.SystemCommandExecutors
2828
import semmle.go.frameworks.SQL
29+
import semmle.go.frameworks.XPath
2930
import semmle.go.frameworks.Stdlib
3031
import semmle.go.security.FlowSources
3132
import semmle.go.Util

‎ql/src/semmle/go/Types.qll

+5
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,11 @@ class SliceType extends @slicetype, CompositeType {
328328
override string toString() { result = "slice type" }
329329
}
330330

331+
/** A byte slice type */
332+
class ByteSliceType extends SliceType {
333+
ByteSliceType() { this.getElementType() instanceof Uint8Type }
334+
}
335+
331336
/** A struct type. */
332337
class StructType extends @structtype, CompositeType {
333338
/**

0 commit comments

Comments
 (0)