Skip to content

Commit 1a7927d

Browse files
committed
Fix x/net/html.EscapeString modelling
This had never worked due to accidentally extending non-abstract class HtmlEscapeFunction; consequently it was neither a taint propagator in general, nor an HTML escape function. Added tests to ensure it is now behaving as intended.
1 parent fa4145b commit 1a7927d

File tree

4 files changed

+15
-1
lines changed

4 files changed

+15
-1
lines changed

go/ql/lib/ext/golang.org.x.net.html.model.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ extensions:
33
pack: codeql/go-all
44
extensible: summaryModel
55
data:
6+
- ["golang.org/x/net/$ANYVERSION/html", "", False, "EscapeString", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
67
- ["golang.org/x/net/$ANYVERSION/html", "", False, "NewTokenizer", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
78
- ["golang.org/x/net/$ANYVERSION/html", "", False, "NewTokenizerFragment", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
89
- ["golang.org/x/net/$ANYVERSION/html", "", False, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,10 @@ import go
1313
module XNetHtml {
1414
/** Gets the package name `golang.org/x/net/html`. */
1515
string packagePath() { result = package("golang.org/x/net", "html") }
16+
17+
private class EscapeString extends EscapeFunction::Range {
18+
EscapeString() { this.hasQualifiedName(packagePath(), "EscapeString") }
19+
20+
override string kind() { result = "html" }
21+
}
1622
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-089/SqlInjection.ql

go/ql/test/library-tests/semmle/go/frameworks/XNetHtml/test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
package test
22

33
import (
4+
"database/sql"
45
"golang.org/x/net/html"
56
"net/http"
67
)
78

89
func test(request *http.Request, writer http.ResponseWriter) {
910

1011
cookie, _ := request.Cookie("SomeCookie")
11-
1212
writer.Write([]byte(html.EscapeString(cookie.Value))) // GOOD: escaped.
1313

1414
writer.Write([]byte(html.UnescapeString(cookie.Value))) // BAD: unescaped.
@@ -49,3 +49,9 @@ func test(request *http.Request, writer http.ResponseWriter) {
4949
html.Render(writer, &cleanNode2) // BAD: writing unescaped HTML data
5050

5151
}
52+
53+
func sqlTest(request *http.Request, db *sql.DB) {
54+
// Ensure EscapeString is a taint propagator for non-XSS queries, e.g. SQL injection:
55+
cookie, _ := request.Cookie("SomeCookie")
56+
db.Query(html.EscapeString(cookie.Value))
57+
}

0 commit comments

Comments
 (0)