Skip to content

Go: convert request-forgery, xpath-injection and credentials sinks to MaD #17072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
owen-mc merged 7 commits into from
Jul 30, 2024
Merged

Go: convert request-forgery, xpath-injection and credentials sinks to MaD #17072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f7d6815
Allow MaD sinks for go/request-forgery
owen-mc Jul 19, 2024
93c9910
Convert go/request-forgery sinks to MaD
owen-mc Jul 19, 2024
78b66ab
Convert existing credentials sinks to MaD
owen-mc Jul 25, 2024
f3069c8
Improve XPath injection test (incl extra sinks)
owen-mc Jul 25, 2024
a6cb511
Convert XPath injection sinks to MaD
owen-mc Jul 25, 2024
1aa63c3
Accept model numbering changes
owen-mc Jul 25, 2024
9cb01d4
Merge branch 'main' into go/mad/convert-sinks
owen-mc Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions go/ql/lib/ext/github.com.antchfx.htmlquery.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/antchfx/htmlquery", "", True, "Find", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/htmlquery", "", True, "FindOne", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/htmlquery", "", True, "Query", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/htmlquery", "", True, "QueryAll", "", "", "Argument[1]", "xpath-injection", "manual"]
9 changes: 9 additions & 0 deletions go/ql/lib/ext/github.com.antchfx.jsonquery.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/antchfx/jsonquery", "", True, "Find", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/jsonquery", "", True, "FindOne", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/jsonquery", "", True, "Query", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/jsonquery", "", True, "QueryAll", "", "", "Argument[1]", "xpath-injection", "manual"]
13 changes: 13 additions & 0 deletions go/ql/lib/ext/github.com.antchfx.xmlquery.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/antchfx/xmlquery", "", True, "Find", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "", True, "FindOne", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "", True, "FindEach", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "", True, "FindEachWithBreak", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "", True, "Query", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "", True, "QueryAll", "", "", "Argument[1]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "Node", True, "SelectElement", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/antchfx/xmlquery", "Node", True, "SelectElements", "", "", "Argument[0]", "xpath-injection", "manual"]
9 changes: 9 additions & 0 deletions go/ql/lib/ext/github.com.antchfx.xpath.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/antchfx/xpath", "", True, "Compile", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/antchfx/xpath", "", True, "CompileWithNS", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/antchfx/xpath", "", True, "MustCompile", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/antchfx/xpath", "", True, "Select", "", "", "Argument[1]", "xpath-injection", "manual"]
6 changes: 6 additions & 0 deletions go/ql/lib/ext/github.com.appleboy.gin-jwt.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/appleboy/gin-jwt", "GinJWTMiddleware", True, "Key", "", "", "", "credentials-key", "manual"]
8 changes: 8 additions & 0 deletions go/ql/lib/ext/github.com.christrenkamp.goxpath.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/ChrisTrenkamp/goxpath", "", True, "MustParse", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/ChrisTrenkamp/goxpath", "", True, "Parse", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/ChrisTrenkamp/goxpath", "", True, "ParseExec", "", "", "Argument[0]", "xpath-injection", "manual"]
14 changes: 14 additions & 0 deletions go/ql/lib/ext/github.com.go-jose.go-jose.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: packageGrouping
data:
- ["go-jose", "github.com/go-jose/go-jose"]
- ["go-jose", "gopkg.in/square/go-jose"]
- ["go-jose", "github.com/square/go-jose"]
Comment on lines +6 to +8
Copy link
Contributor

@egregius313 egregius313 Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also handle "gopkg.in/square/go-jose.v2"? Just double checking that the .v2 is ok.

Copy link
Contributor Author

@owen-mc owen-mc Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this regex matches .v2 or /v2, so then this predicate removes either, and that is used here on the imported package name to match it against the string given in the MaD yml file.

This test imports it in that way and it still passes on this PR.

Copy link
Contributor Author

@owen-mc owen-mc Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun fact: I found out recently that the .v2 format is only for gopkg.in. I guess that site used that syntax before go modules were invented and it was easier to accept it than to make everyone change their imports.

- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["group:go-jose", "Recipient", True, "Key", "", "", "", "credentials-key", "manual"]
- ["group:go-jose", "SigningKey", True, "Key", "", "", "", "credentials-key", "manual"]
7 changes: 7 additions & 0 deletions go/ql/lib/ext/github.com.go-xmlpath.xmlpath.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/go-xmlpath/xmlpath", "", True, "Compile", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/go-xmlpath/xmlpath", "", True, "MustCompile", "", "", "Argument[0]", "xpath-injection", "manual"]
6 changes: 6 additions & 0 deletions go/ql/lib/ext/github.com.gogf.gf-jwt.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/gogf/gf-jwt", "GfJWTMiddleware", True, "Key", "", "", "", "credentials-key", "manual"]
6 changes: 6 additions & 0 deletions go/ql/lib/ext/github.com.jbowtie.gokogiri.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/jbowtie/gokogiri/xpath", "", True, "Compile", "", "", "Argument[0]", "xpath-injection", "manual"]
9 changes: 9 additions & 0 deletions go/ql/lib/ext/github.com.jbowtie.gokogiri.xml.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/jbowtie/gokogiri/xml", "Node", True, "Search", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/jbowtie/gokogiri/xml", "Node", True, "SearchWithVariables", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/jbowtie/gokogiri/xml", "Node", True, "EvalXPath", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/jbowtie/gokogiri/xml", "Node", True, "EvalXPathAsBoolean", "", "", "Argument[0]", "xpath-injection", "manual"]
1 change: 1 addition & 0 deletions go/ql/lib/ext/github.com.kataras.iris.middleware.jwt.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ extensions:
extensible: sinkModel
data:
- ["github.com/kataras/iris/middleware/jwt", "", True, "NewSigner", "", "", "Argument[1]", "credentials-key", "manual"]
- ["github.com/kataras/iris/middleware/jwt", "Signer", True, "Key", "", "", "", "credentials-key", "manual"]
8 changes: 8 additions & 0 deletions go/ql/lib/ext/github.com.lestrrat-go.libxml2.parser.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/lestrrat-go/libxml2/parser", "Parser", True, "Parse", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/lestrrat-go/libxml2/parser", "Parser", True, "ParseReader", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/lestrrat-go/libxml2/parser", "Parser", True, "ParseString", "", "", "Argument[0]", "xpath-injection", "manual"]
7 changes: 7 additions & 0 deletions go/ql/lib/ext/github.com.santhosh-tekuri.xpathparser.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/santhosh-tekuri/xpathparser", "", True, "Parse", "", "", "Argument[0]", "xpath-injection", "manual"]
- ["github.com/santhosh-tekuri/xpathparser", "", True, "MustParse", "", "", "Argument[0]", "xpath-injection", "manual"]
30 changes: 29 additions & 1 deletion go/ql/lib/ext/github.com.valyala.fasthttp.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,33 @@
extensions:
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data:
- ["github.com/valyala/fasthttp", "", True, "Get", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "", True, "GetDeadline", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "", True, "GetTimeout", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "", True, "Post", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "", True, "Dial", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "", True, "DialDualStack", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "", True, "DialDualStackTimeout", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "", True, "DialTimeout", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "Client", True, "Get", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Client", True, "GetDeadline", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Client", True, "GetTimeout", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Client", True, "Post", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "HostClient", True, "Get", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "HostClient", True, "GetDeadline", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "HostClient", True, "GetTimeout", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "HostClient", True, "Post", "", "", "Argument[1]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Request", True, "SetHost", "", "", "Argument[0]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Request", True, "SetHostBytes", "", "", "Argument[0]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Request", True, "SetRequestURI", "", "", "Argument[0]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Request", True, "SetRequestURIBytes", "", "", "Argument[0]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "Request", True, "SetURI", "", "", "Argument[0]", "request-forgery", "manual"]
- ["github.com/valyala/fasthttp", "TCPDialer", True, "Dial", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "TCPDialer", True, "DialDualStack", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "TCPDialer", True, "DialDualStackTimeout", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- ["github.com/valyala/fasthttp", "TCPDialer", True, "DialTimeout", "", "", "Argument[0]", "request-forgery[TCP Addr + Port]", "manual"]
- addsTo:
pack: codeql/go-all
extensible: summaryModel
Expand All @@ -8,7 +37,6 @@ extensions:
- ["github.com/valyala/fasthttp", "URI", False, "Update", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"]
- ["github.com/valyala/fasthttp", "URI", False, "UpdateBytes", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"]
- ["github.com/valyala/fasthttp", "URI", False, "Parse", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]

- addsTo:
pack: codeql/go-all
extensible: sourceModel
Expand Down
1 change: 0 additions & 1 deletion go/ql/lib/go.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import semmle.go.frameworks.Fiber
import semmle.go.frameworks.Gin
import semmle.go.frameworks.GinCors
import semmle.go.frameworks.Glog
import semmle.go.frameworks.Gogf
import semmle.go.frameworks.GoJose
import semmle.go.frameworks.GoKit
import semmle.go.frameworks.GoMicro
Expand Down
36 changes: 27 additions & 9 deletions go/ql/lib/semmle/go/frameworks/Fasthttp.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,15 @@ module Fasthttp {
}

/**
* DEPRECATED: Use `RequestForgery::Sink` instead.
*
* A function that sends HTTP requests.
*
* Get* send a HTTP GET request.
* Post send a HTTP POST request.
* These functions first argument is a URL.
*/
class RequestForgerySink extends RequestForgery::Sink {
deprecated class RequestForgerySink extends RequestForgery::Sink {
RequestForgerySink() {
exists(Function f |
f.hasQualifiedName(packagePath(), ["Get", "GetDeadline", "GetTimeout", "Post"]) and
Expand All @@ -233,10 +235,12 @@ module Fasthttp {
}

/**
* DEPRECATED: Use `RequestForgery::Sink` instead.
*
* A function that create initial connection to a TCP address.
* Following Functions only accept TCP address + Port in their first argument.
*/
class RequestForgerySinkDial extends RequestForgery::Sink {
deprecated class RequestForgerySinkDial extends RequestForgery::Sink {
RequestForgerySinkDial() {
exists(Function f |
f.hasQualifiedName(packagePath(),
Expand Down Expand Up @@ -308,15 +312,19 @@ module Fasthttp {
}

/**
* DEPRECATED
*
* Provide modeling for fasthttp.TCPDialer Type.
*/
module TcpDialer {
deprecated module TcpDialer {
/**
* DEPRECATED: Use `RequestForgery::Sink` instead.
*
* A method that create initial connection to a TCP address.
* Provide Methods which can be used as dangerous RequestForgery Sinks.
* Following Methods only accept TCP address + Port in their first argument.
*/
class RequestForgerySinkDial extends RequestForgery::Sink {
deprecated class RequestForgerySinkDial extends RequestForgery::Sink {
RequestForgerySinkDial() {
exists(Method m |
m.hasQualifiedName(packagePath(), "TCPDialer",
Expand All @@ -332,16 +340,20 @@ module Fasthttp {
}

/**
* DEPRECATED
*
* Provide modeling for fasthttp.Client Type.
*/
module Client {
deprecated module Client {
/**
* DEPRECATED: Use `RequestForgery::Sink` instead.
*
* A method that sends HTTP requests.
* Get* send a HTTP GET request.
* Post send a HTTP POST request.
* these Functions first arguments is a URL.
*/
class RequestForgerySink extends RequestForgery::Sink {
deprecated class RequestForgerySink extends RequestForgery::Sink {
RequestForgerySink() {
exists(Method m |
m.hasQualifiedName(packagePath(), "Client", ["Get", "GetDeadline", "GetTimeout", "Post"]) and
Expand All @@ -356,16 +368,20 @@ module Fasthttp {
}

/**
* DEPRECATED
*
* Provide modeling for fasthttp.HostClient Type.
*/
module HostClient {
deprecated module HostClient {
/**
* DEPRECATED: Use `RequestForgery::Sink` instead.
*
* A method that sends HTTP requests.
* Get* send a HTTP GET request.
* Post send a HTTP POST request.
* these Functions first arguments is a URL.
*/
class RequestForgerySink extends RequestForgery::Sink {
deprecated class RequestForgerySink extends RequestForgery::Sink {
RequestForgerySink() {
exists(Method m |
m.hasQualifiedName(packagePath(), "HostClient",
Expand Down Expand Up @@ -434,12 +450,14 @@ module Fasthttp {
}

/**
* DEPRECATED: Use `RequestForgery::Sink` instead.
*
* A method that create the URL and Host parts of a `Request` type.
*
* This instance of `Request` type can be used in some functions/methods
* like `func Do(req *Request, resp *Response) error` that will lead to server side request forgery vulnerability.
*/
class RequestForgerySink extends RequestForgery::Sink {
deprecated class RequestForgerySink extends RequestForgery::Sink {
RequestForgerySink() {
exists(Method m |
m.hasQualifiedName(packagePath(), "Request",
Expand Down
10 changes: 0 additions & 10 deletions go/ql/lib/semmle/go/frameworks/Gin.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/

import go
private import semmle.go.security.HardcodedCredentials

private module Gin {
/** Gets the package name `github.com/gin-gonic/gin`. */
Expand All @@ -30,13 +29,4 @@ private module Gin {

override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
}

private class GinJwtSign extends HardcodedCredentials::Sink {
GinJwtSign() {
exists(Field f |
f.hasQualifiedName(package("github.com/appleboy/gin-jwt", ""), "GinJWTMiddleware", "Key") and
f.getAWrite().getRhs() = this
)
}
}
}
18 changes: 0 additions & 18 deletions go/ql/lib/semmle/go/frameworks/GoJose.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,8 @@
*/

import go
private import semmle.go.security.HardcodedCredentials

private module GoJose {
private class GoJoseKey extends HardcodedCredentials::Sink {
GoJoseKey() {
exists(Field f |
f.hasQualifiedName(goJosePackage(), ["Recipient", "SigningKey"], "Key") and
f.getAWrite().getRhs() = this
)
}
}

private string goJosePackage() {
result =
[
package("github.com/square/go-jose", ""), package("github.com/go-jose/go-jose", ""),
"gopkg.in/square/go-jose.v2"
]
}

/**
* Provides classes and predicates for working with the `gopkg.in/square/go-jose/jwt` and
* `github.com/go-jose/go-jose/jwt` packages.
Expand Down
17 changes: 0 additions & 17 deletions go/ql/lib/semmle/go/frameworks/Gogf.qll
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions go/ql/lib/semmle/go/frameworks/Iris.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/

import go
private import semmle.go.security.HardcodedCredentials

private module Iris {
/** Gets the v1 module path `github.com/kataras/iris`. */
Expand Down Expand Up @@ -47,13 +46,4 @@ private module Iris {

override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
}

private class IrisJwt extends HardcodedCredentials::Sink {
IrisJwt() {
exists(Field f |
f.hasQualifiedName(package("github.com/kataras/iris", "middleware/jwt"), "Signer", "Key") and
f.getAWrite().getRhs() = this
)
}
}
}
Loading