Skip to content

Commit 19c5d1f

Browse files
authored
Merge pull request #15181 from felickz/go-xxe-libxml2
GO - Add sink for libxml2 in go/xml/xpath-injection via XPath.qll
2 parents bbe3269 + 730f6ed commit 19c5d1f

File tree

7 files changed

+264
-140
lines changed

7 files changed

+264
-140
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The XPath library, which is used for the XPath injection query (`go/xml/xpath-injection`), now includes support for `Parser` sinks from the [libxml2](https://github.com/lestrrat-go/libxml2) package.

go/ql/lib/semmle/go/frameworks/XPath.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,19 @@ module XPath {
142142
}
143143
}
144144

145+
/**
146+
* An XPath expression string used in an API function of the
147+
* [lestrrat-go/libxml2](https://github.com/lestrrat-go/libxml2) package.
148+
*/
149+
private class LestratGoLibxml2XPathExpressionString extends Range {
150+
LestratGoLibxml2XPathExpressionString() {
151+
exists(Method m, string name | name.matches("Parse%") |
152+
m.hasQualifiedName(package("github.com/lestrrat-go/libxml2", "parser"), "Parser", name) and
153+
this = m.getACall().getArgument(0)
154+
)
155+
}
156+
}
157+
145158
/**
146159
* An XPath expression string used in an API function of the
147160
* [xpathparser](https://github.com/santhosh-tekuri/xpathparser) package.

go/ql/test/query-tests/Security/CWE-643/XPathInjection.expected

Lines changed: 145 additions & 139 deletions
Large diffs are not rendered by default.

go/ql/test/query-tests/Security/CWE-643/go.mod

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module main
22

3-
go 1.14
3+
go 1.21
44

55
require (
66
github.com/ChrisTrenkamp/goxpath v0.0.0-20190607011252-c5096ec8773d
@@ -10,5 +10,19 @@ require (
1010
github.com/antchfx/xpath v1.1.5
1111
github.com/go-xmlpath/xmlpath v0.0.0-20150820204837-860cbeca3ebc
1212
github.com/jbowtie/gokogiri v0.0.0-20190301021639-37f655d3078f
13+
github.com/lestrrat-go/libxml2 v0.0.0-20231124114421-99c71026c2f5
1314
github.com/santhosh-tekuri/xpathparser v1.0.0
1415
)
16+
17+
require (
18+
github.com/davecgh/go-spew v1.1.1 // indirect
19+
github.com/pkg/errors v0.9.1 // indirect
20+
github.com/pmezard/go-difflib v1.0.0 // indirect
21+
github.com/stretchr/objx v0.5.0 // indirect
22+
github.com/stretchr/testify v1.8.4 // indirect
23+
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 // indirect
24+
gopkg.in/xmlpath.v1 v1.0.0-20140413065638-a146725ea6e7 // indirect
25+
gopkg.in/yaml.v3 v3.0.1 // indirect
26+
launchpad.net/gocheck v0.0.0-20140225173054-000000000087 // indirect
27+
launchpad.net/xmlpath v0.0.0-20130614043138-000000000004 // indirect
28+
)

go/ql/test/query-tests/Security/CWE-643/tst.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ package main
1010
//go:generate depstubber -vendor github.com/jbowtie/gokogiri/xml Node
1111
//go:generate depstubber -vendor github.com/jbowtie/gokogiri/xpath "" Compile
1212
//go:generate depstubber -vendor github.com/santhosh-tekuri/xpathparser "" Parse,MustParse
13+
//go:generate depstubber -vendor github.com/lestrrat-go/libxml2/parser Parser New,XMLParseNoEnt
1314

1415
import (
1516
"net/http"
@@ -22,6 +23,7 @@ import (
2223
"github.com/go-xmlpath/xmlpath"
2324
gokogiriXml "github.com/jbowtie/gokogiri/xml"
2425
gokogiriXpath "github.com/jbowtie/gokogiri/xpath"
26+
"github.com/lestrrat-go/libxml2/parser"
2527
"github.com/santhosh-tekuri/xpathparser"
2628
)
2729

@@ -185,3 +187,13 @@ func testJbowtieGokogiri(r *http.Request, n gokogiriXml.Node) {
185187
// OK: This is not flagged, since the creation of `xpath` is already flagged.
186188
_ = n.EvalXPathAsBoolean(xpath, nil)
187189
}
190+
191+
func testLestratGoLibxml2(r *http.Request) {
192+
r.ParseForm()
193+
username := r.Form.Get("username")
194+
195+
p := parser.New(parser.XMLParseNoEnt)
196+
197+
// BAD: User input used directly in an XPath expression
198+
_, _ = p.ParseString("//users/user[login/text()='" + username + "']/home_dir/text()")
199+
}

go/ql/test/query-tests/Security/CWE-643/vendor/github.com/lestrrat-go/libxml2/parser/stub.go

Lines changed: 42 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

go/ql/test/query-tests/Security/CWE-643/vendor/modules.txt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,39 @@ github.com/go-xmlpath/xmlpath
1919
# github.com/jbowtie/gokogiri v0.0.0-20190301021639-37f655d3078f
2020
## explicit
2121
github.com/jbowtie/gokogiri
22+
# github.com/lestrrat-go/libxml2 v0.0.0-20231124114421-99c71026c2f5
23+
## explicit
24+
github.com/lestrrat-go/libxml2
2225
# github.com/santhosh-tekuri/xpathparser v1.0.0
2326
## explicit
2427
github.com/santhosh-tekuri/xpathparser
28+
# github.com/davecgh/go-spew v1.1.1
29+
## explicit
30+
github.com/davecgh/go-spew
31+
# github.com/pkg/errors v0.9.1
32+
## explicit
33+
github.com/pkg/errors
34+
# github.com/pmezard/go-difflib v1.0.0
35+
## explicit
36+
github.com/pmezard/go-difflib
37+
# github.com/stretchr/objx v0.5.0
38+
## explicit
39+
github.com/stretchr/objx
40+
# github.com/stretchr/testify v1.8.4
41+
## explicit
42+
github.com/stretchr/testify
43+
# gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405
44+
## explicit
45+
gopkg.in/check.v1
46+
# gopkg.in/xmlpath.v1 v1.0.0-20140413065638-a146725ea6e7
47+
## explicit
48+
gopkg.in/xmlpath.v1
49+
# gopkg.in/yaml.v3 v3.0.1
50+
## explicit
51+
gopkg.in/yaml.v3
52+
# launchpad.net/gocheck v0.0.0-20140225173054-000000000087
53+
## explicit
54+
launchpad.net/gocheck
55+
# launchpad.net/xmlpath v0.0.0-20130614043138-000000000004
56+
## explicit
57+
launchpad.net/xmlpath

0 commit comments

Comments
 (0)