Go: convert request-forgery, xpath-injection and credentials sinks to MaD by owen-mc · Pull Request #17072 · github/codeql

owen-mc · 2024-07-25T11:52:46Z

Convert request-forgery, xpath-injection and credentials sinks to use MaD.

Request forgery sinks which have `getRequest` different from the sink itself cannot be modeled using models-as-data.

I checked that the tests failed when I removed the classes and passed again when I add the MaD models.

Currently the extra sinks are not detected. This will be fixed in the next commit.

github-actions · 2024-07-25T11:59:36Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

go

Generated file changes for go

Changes to framework-coverage-go.rst:

-    `Go JOSE <https://github.com/go-jose/go-jose>`_,"``github.com/go-jose/go-jose*``, ``github.com/square/go-jose*``, ``gopkg.in/square/go-jose*``",,12,3
+    `Go JOSE <https://github.com/go-jose/go-jose>`_,"``github.com/go-jose/go-jose*``, ``github.com/square/go-jose*``, ``gopkg.in/square/go-jose*``",,12,9
-    `Iris <https://www.iris-go.com/>`_,``github.com/kataras/iris*``,,,1
+    `Iris <https://www.iris-go.com/>`_,``github.com/kataras/iris*``,,,2
-    `fasthttp <https://github.com/valyala/fasthttp>`_,``github.com/valyala/fasthttp*``,50,5,
+    `fasthttp <https://github.com/valyala/fasthttp>`_,``github.com/valyala/fasthttp*``,50,5,25
-    Others,"``github.com/gobwas/ws``, ``github.com/gorilla/websocket``, ``nhooyr.io/websocket``",7,,
+    Others,"``github.com/ChrisTrenkamp/goxpath``, ``github.com/antchfx/htmlquery``, ``github.com/antchfx/jsonquery``, ``github.com/antchfx/xmlquery``, ``github.com/antchfx/xpath``, ``github.com/appleboy/gin-jwt``, ``github.com/go-xmlpath/xmlpath``, ``github.com/gobwas/ws``, ``github.com/gogf/gf-jwt``, ``github.com/gorilla/websocket``, ``github.com/jbowtie/gokogiri/xml``, ``github.com/jbowtie/gokogiri/xpath``, ``github.com/lestrrat-go/libxml2/parser``, ``github.com/santhosh-tekuri/xpathparser``, ``nhooyr.io/websocket``",7,,37
-    Totals,,267,902,25
+    Totals,,267,902,94

Changes to framework-coverage-go.csv:

- package,sink,source,summary,sink:credentials-key,sink:jwt,source:remote,summary:taint,summary:value
+ package,sink,source,summary,sink:credentials-key,sink:jwt,sink:request-forgery,sink:request-forgery[TCP Addr + Port],sink:xpath-injection,source:remote,summary:taint,summary:value
- ,,,8,,,,3,5
+ ,,,8,,,,,,,3,5
- archive/tar,,,5,,,,5,
+ archive/tar,,,5,,,,,,,5,
- archive/zip,,,6,,,,6,
+ archive/zip,,,6,,,,,,,6,
- bufio,,,17,,,,17,
+ bufio,,,17,,,,,,,17,
- bytes,,,43,,,,43,
+ bytes,,,43,,,,,,,43,
- compress/bzip2,,,1,,,,1,
+ compress/bzip2,,,1,,,,,,,1,
- compress/flate,,,4,,,,4,
+ compress/flate,,,4,,,,,,,4,
- compress/gzip,,,3,,,,3,
+ compress/gzip,,,3,,,,,,,3,
- compress/lzw,,,1,,,,1,
+ compress/lzw,,,1,,,,,,,1,
- compress/zlib,,,4,,,,4,
+ compress/zlib,,,4,,,,,,,4,
- container/heap,,,5,,,,5,
+ container/heap,,,5,,,,,,,5,
- container/list,,,20,,,,20,
+ container/list,,,20,,,,,,,20,
- container/ring,,,5,,,,5,
+ container/ring,,,5,,,,,,,5,
- context,,,5,,,,5,
+ context,,,5,,,,,,,5,
- crypto,,,1,,,,1,
+ crypto,,,1,,,,,,,1,
- crypto/cipher,,,3,,,,3,
+ crypto/cipher,,,3,,,,,,,3,
- crypto/rsa,,,2,,,,2,
+ crypto/rsa,,,2,,,,,,,2,
- crypto/tls,,,3,,,,3,
+ crypto/tls,,,3,,,,,,,3,
- crypto/x509,,,1,,,,1,
+ crypto/x509,,,1,,,,,,,1,
- database/sql,,,7,,,,7,
+ database/sql,,,7,,,,,,,7,
- database/sql/driver,,,4,,,,4,
+ database/sql/driver,,,4,,,,,,,4,
- encoding,,,4,,,,4,
+ encoding,,,4,,,,,,,4,
- encoding/ascii85,,,2,,,,2,
+ encoding/ascii85,,,2,,,,,,,2,
- encoding/asn1,,,8,,,,8,
+ encoding/asn1,,,8,,,,,,,8,
- encoding/base32,,,3,,,,3,
+ encoding/base32,,,3,,,,,,,3,
- encoding/base64,,,3,,,,3,
+ encoding/base64,,,3,,,,,,,3,
- encoding/binary,,,2,,,,2,
+ encoding/binary,,,2,,,,,,,2,
- encoding/csv,,,5,,,,5,
+ encoding/csv,,,5,,,,,,,5,
- encoding/gob,,,7,,,,7,
+ encoding/gob,,,7,,,,,,,7,
- encoding/hex,,,3,,,,3,
+ encoding/hex,,,3,,,,,,,3,
- encoding/json,,,14,,,,14,
+ encoding/json,,,14,,,,,,,14,
- encoding/pem,,,3,,,,3,
+ encoding/pem,,,3,,,,,,,3,
- encoding/xml,,,23,,,,23,
+ encoding/xml,,,23,,,,,,,23,
- errors,,,3,,,,3,
+ errors,,,3,,,,,,,3,
- expvar,,,6,,,,6,
+ expvar,,,6,,,,,,,6,
- fmt,,,16,,,,16,
+ fmt,,,16,,,,,,,16,
+ github.com/ChrisTrenkamp/goxpath,3,,,,,,,3,,,
+ github.com/antchfx/htmlquery,4,,,,,,,4,,,
+ github.com/antchfx/jsonquery,4,,,,,,,4,,,
+ github.com/antchfx/xmlquery,8,,,,,,,8,,,
+ github.com/antchfx/xpath,4,,,,,,,4,,,
+ github.com/appleboy/gin-jwt,1,,,1,,,,,,,
- github.com/astaxie/beego,,6,7,,,6,7,
+ github.com/astaxie/beego,,6,7,,,,,,6,7,
- github.com/astaxie/beego/context,,15,1,,,15,1,
+ github.com/astaxie/beego/context,,15,1,,,,,,15,1,
- github.com/astaxie/beego/utils,,,13,,,,13,
+ github.com/astaxie/beego/utils,,,13,,,,,,,13,
- github.com/beego/beego,,6,7,,,6,7,
+ github.com/beego/beego,,6,7,,,,,,6,7,
- github.com/beego/beego/context,,15,1,,,15,1,
+ github.com/beego/beego/context,,15,1,,,,,,15,1,
- github.com/beego/beego/core/utils,,,13,,,,13,
+ github.com/beego/beego/core/utils,,,13,,,,,,,13,
- github.com/beego/beego/server/web,,6,7,,,6,7,
+ github.com/beego/beego/server/web,,6,7,,,,,,6,7,
- github.com/beego/beego/server/web/context,,15,1,,,15,1,
+ github.com/beego/beego/server/web/context,,15,1,,,,,,15,1,
- github.com/beego/beego/utils,,,13,,,,13,
+ github.com/beego/beego/utils,,,13,,,,,,,13,
- github.com/couchbase/gocb,,,18,,,,18,
+ github.com/couchbase/gocb,,,18,,,,,,,18,
- github.com/couchbaselabs/gocb,,,18,,,,18,
+ github.com/couchbaselabs/gocb,,,18,,,,,,,18,
- github.com/cristalhq/jwt,1,,,1,,,,
+ github.com/cristalhq/jwt,1,,,1,,,,,,,
- github.com/dgrijalva/jwt-go,3,,9,2,1,,9,
+ github.com/dgrijalva/jwt-go,3,,9,2,1,,,,,9,
- github.com/elazarl/goproxy,,2,2,,,2,2,
+ github.com/elazarl/goproxy,,2,2,,,,,,2,2,
- github.com/emicklei/go-restful,,7,,,,7,,
+ github.com/emicklei/go-restful,,7,,,,,,,7,,
- github.com/evanphx/json-patch,,,12,,,,12,
+ github.com/evanphx/json-patch,,,12,,,,,,,12,
- github.com/form3tech-oss/jwt-go,2,,,2,,,,
+ github.com/form3tech-oss/jwt-go,2,,,2,,,,,,,
- github.com/gin-gonic/gin,,46,2,,,46,2,
+ github.com/gin-gonic/gin,,46,2,,,,,,46,2,
- github.com/go-chi/chi,,3,,,,3,,
+ github.com/go-chi/chi,,3,,,,,,,3,,
- github.com/go-chi/jwtauth,1,,,1,,,,
+ github.com/go-chi/jwtauth,1,,,1,,,,,,,
+ github.com/go-jose/go-jose,2,,,2,,,,,,,
- github.com/go-jose/go-jose/jwt,1,,4,,1,,4,
+ github.com/go-jose/go-jose/jwt,1,,4,,1,,,,,4,
- github.com/go-kit/kit/auth/jwt,1,,,1,,,,
+ github.com/go-kit/kit/auth/jwt,1,,,1,,,,,,,
- github.com/go-pg/pg/orm,,,6,,,,6,
+ github.com/go-pg/pg/orm,,,6,,,,,,,6,
+ github.com/go-xmlpath/xmlpath,2,,,,,,,2,,,
- github.com/gobwas/ws,,2,,,,2,,
+ github.com/gobwas/ws,,2,,,,,,,2,,
+ github.com/gogf/gf-jwt,1,,,1,,,,,,,
- github.com/golang-jwt/jwt,3,,11,2,1,,11,
+ github.com/golang-jwt/jwt,3,,11,2,1,,,,,11,
- github.com/golang/protobuf/proto,,,4,,,,4,
+ github.com/golang/protobuf/proto,,,4,,,,,,,4,
- github.com/gorilla/mux,,1,,,,1,,
+ github.com/gorilla/mux,,1,,,,,,,1,,
- github.com/gorilla/websocket,,3,,,,3,,
+ github.com/gorilla/websocket,,3,,,,,,,3,,
+ github.com/jbowtie/gokogiri/xml,4,,,,,,,4,,,
+ github.com/jbowtie/gokogiri/xpath,1,,,,,,,1,,,
- github.com/json-iterator/go,,,4,,,,4,
+ github.com/json-iterator/go,,,4,,,,,,,4,
- github.com/kataras/iris/middleware/jwt,1,,,1,,,,
+ github.com/kataras/iris/middleware/jwt,2,,,2,,,,,,,
- github.com/kataras/jwt,5,,,5,,,,
+ github.com/kataras/jwt,5,,,5,,,,,,,
- github.com/labstack/echo,,12,2,,,12,2,
+ github.com/labstack/echo,,12,2,,,,,,12,2,
- github.com/lestrrat-go/jwx,1,,,1,,,,
+ github.com/lestrrat-go/jwx,1,,,1,,,,,,,
- github.com/lestrrat-go/jwx/jwk,1,,,1,,,,
+ github.com/lestrrat-go/jwx/jwk,1,,,1,,,,,,,
+ github.com/lestrrat-go/libxml2/parser,3,,,,,,,3,,,
- github.com/lestrrat/go-jwx/jwk,1,,,1,,,,
+ github.com/lestrrat/go-jwx/jwk,1,,,1,,,,,,,
- github.com/ory/fosite/token/jwt,2,,,2,,,,
+ github.com/ory/fosite/token/jwt,2,,,2,,,,,,,
- github.com/revel/revel,,23,10,,,23,10,
+ github.com/revel/revel,,23,10,,,,,,23,10,
- github.com/robfig/revel,,23,10,,,23,10,
+ github.com/robfig/revel,,23,10,,,,,,23,10,
+ github.com/santhosh-tekuri/xpathparser,2,,,,,,,2,,,
- github.com/sendgrid/sendgrid-go/helpers/mail,,,1,,,,1,
+ github.com/sendgrid/sendgrid-go/helpers/mail,,,1,,,,,,,1,
+ github.com/square/go-jose,2,,,2,,,,,,,
- github.com/square/go-jose/jwt,1,,4,,1,,4,
+ github.com/square/go-jose/jwt,1,,4,,1,,,,,4,
- github.com/valyala/fasthttp,,50,5,,,50,5,
+ github.com/valyala/fasthttp,25,50,5,,,17,8,,50,5,
- go.uber.org/zap,,,11,,,,11,
+ go.uber.org/zap,,,11,,,,,,,11,
- golang.org/x/net/context,,,5,,,,5,
+ golang.org/x/net/context,,,5,,,,,,,5,
- golang.org/x/net/html,,,16,,,,16,
+ golang.org/x/net/html,,,16,,,,,,,16,
- golang.org/x/net/websocket,,2,,,,2,,
+ golang.org/x/net/websocket,,2,,,,,,,2,,
- google.golang.org/protobuf/internal/encoding/text,,,1,,,,1,
+ google.golang.org/protobuf/internal/encoding/text,,,1,,,,,,,1,
- google.golang.org/protobuf/internal/impl,,,2,,,,2,
+ google.golang.org/protobuf/internal/impl,,,2,,,,,,,2,
- google.golang.org/protobuf/proto,,,8,,,,8,
+ google.golang.org/protobuf/proto,,,8,,,,,,,8,
- google.golang.org/protobuf/reflect/protoreflect,,,1,,,,1,
+ google.golang.org/protobuf/reflect/protoreflect,,,1,,,,,,,1,
- gopkg.in/couchbase/gocb,,,18,,,,18,
+ gopkg.in/couchbase/gocb,,,18,,,,,,,18,
- gopkg.in/macaron,,12,1,,,12,1,
+ gopkg.in/macaron,,12,1,,,,,,12,1,
+ gopkg.in/square/go-jose,2,,,2,,,,,,,
- gopkg.in/square/go-jose/jwt,1,,4,,1,,4,
+ gopkg.in/square/go-jose/jwt,1,,4,,1,,,,,4,
- gopkg.in/yaml,,,9,,,,9,
+ gopkg.in/yaml,,,9,,,,,,,9,
- html,,,2,,,,2,
+ html,,,2,,,,,,,2,
- html/template,,,6,,,,6,
+ html/template,,,6,,,,,,,6,
- io,,,19,,,,19,
+ io,,,19,,,,,,,19,
- io/fs,,,12,,,,12,
+ io/fs,,,12,,,,,,,12,
- io/ioutil,,,2,,,,2,
+ io/ioutil,,,2,,,,,,,2,
- k8s.io/api/core,,,10,,,,10,
+ k8s.io/api/core,,,10,,,,,,,10,
- k8s.io/apimachinery/pkg/runtime,,,47,,,,47,
+ k8s.io/apimachinery/pkg/runtime,,,47,,,,,,,47,
- log,,,3,,,,3,
+ log,,,3,,,,,,,3,
- math/big,,,1,,,,1,
+ math/big,,,1,,,,,,,1,
- mime,,,5,,,,5,
+ mime,,,5,,,,,,,5,
- mime/multipart,,,8,,,,8,
+ mime/multipart,,,8,,,,,,,8,
- mime/quotedprintable,,,1,,,,1,
+ mime/quotedprintable,,,1,,,,,,,1,
- net,,,20,,,,20,
+ net,,,20,,,,,,,20,
- net/http,,16,22,,,16,22,
+ net/http,,16,22,,,,,,16,22,
- net/http/httputil,,,10,,,,10,
+ net/http/httputil,,,10,,,,,,,10,
- net/mail,,,6,,,,6,
+ net/mail,,,6,,,,,,,6,
- net/textproto,,,19,,,,19,
+ net/textproto,,,19,,,,,,,19,
- net/url,,,23,,,,23,
+ net/url,,,23,,,,,,,23,
- nhooyr.io/websocket,,2,,,,2,,
+ nhooyr.io/websocket,,2,,,,,,,2,,
- os,,,4,,,,4,
+ os,,,4,,,,,,,4,
- path,,,5,,,,5,
+ path,,,5,,,,,,,5,
- path/filepath,,,13,,,,13,
+ path/filepath,,,13,,,,,,,13,
- reflect,,,37,,,,37,
+ reflect,,,37,,,,,,,37,
- regexp,,,20,,,,20,
+ regexp,,,20,,,,,,,20,
- sort,,,1,,,,1,
+ sort,,,1,,,,,,,1,
- strconv,,,9,,,,9,
+ strconv,,,9,,,,,,,9,
- strings,,,34,,,,34,
+ strings,,,34,,,,,,,34,
- sync,,,10,,,,10,
+ sync,,,10,,,,,,,10,
- sync/atomic,,,24,,,,24,
+ sync/atomic,,,24,,,,,,,24,
- syscall,,,8,,,,8,
+ syscall,,,8,,,,,,,8,
- text/scanner,,,3,,,,3,
+ text/scanner,,,3,,,,,,,3,
- text/tabwriter,,,1,,,,1,
+ text/tabwriter,,,1,,,,,,,1,
- text/template,,,6,,,,6,
+ text/template,,,6,,,,,,,6,

egregius313 · 2024-07-29T20:52:40Z

go/ql/lib/ext/github.com.go-jose.go-jose.model.yml

+      - ["go-jose", "github.com/go-jose/go-jose"]
+      - ["go-jose", "gopkg.in/square/go-jose"]
+      - ["go-jose", "github.com/square/go-jose"]


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

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.

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.

egregius313

As long as the answer to my go-jose.v2 question is that it's fine, this LGTM.

smowton

Proxying @egregius313 review; haven't myself reviewed

github-actions bot added the Go label Jul 25, 2024

owen-mc added 5 commits July 25, 2024 12:53

Allow MaD sinks for go/request-forgery

f7d6815

Request forgery sinks which have `getRequest` different from the sink itself cannot be modeled using models-as-data.

Convert go/request-forgery sinks to MaD

93c9910

Convert existing credentials sinks to MaD

78b66ab

I checked that the tests failed when I removed the classes and passed again when I add the MaD models.

Improve XPath injection test (incl extra sinks)

f3069c8

Currently the extra sinks are not detected. This will be fixed in the next commit.

Convert XPath injection sinks to MaD

a6cb511

owen-mc force-pushed the go/mad/convert-sinks branch from 590936c to a6cb511 Compare July 25, 2024 11:56

Accept model numbering changes

1aa63c3

owen-mc changed the title ~~Go: convert sinks to MaD~~ Go: convert request-forgery, xpath-injection and credentials sinks to MaD Jul 25, 2024

owen-mc added the no-change-note-required This PR does not need a change note label Jul 25, 2024

owen-mc marked this pull request as ready for review July 25, 2024 13:59

owen-mc requested a review from a team as a code owner July 25, 2024 13:59

egregius313 reviewed Jul 29, 2024

View reviewed changes

egregius313 previously approved these changes Jul 29, 2024

View reviewed changes

Merge branch 'main' into go/mad/convert-sinks

9cb01d4

owen-mc dismissed egregius313’s stale review via 9cb01d4 July 30, 2024 07:03

smowton approved these changes Jul 30, 2024

View reviewed changes

owen-mc merged commit 1347f55 into github:main Jul 30, 2024

owen-mc deleted the go/mad/convert-sinks branch July 30, 2024 10:18

owen-mc mentioned this pull request Jul 30, 2024

Go: Use provenance pretty-printing for qlref tests #17092

Merged

owen-mc mentioned this pull request Aug 6, 2024

Go: Convert log-injection, path-injection and command-injection sinks to use models-as-data #17162

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

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

Go: convert request-forgery, xpath-injection and credentials sinks to MaD#17072
owen-mc merged 7 commits intogithub:mainfrom
owen-mc:go/mad/convert-sinks

owen-mc commented Jul 25, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jul 25, 2024

go

Generated file changes for go

Uh oh!

egregius313 Jul 29, 2024

Uh oh!

owen-mc Jul 30, 2024

Uh oh!

owen-mc Jul 30, 2024

Uh oh!

egregius313 left a comment •

edited

Loading

Uh oh!

smowton left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

owen-mc commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 25, 2024

go

Generated file changes for go

Uh oh!

egregius313 Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

owen-mc Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

owen-mc Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

egregius313 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smowton left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

owen-mc commented Jul 25, 2024 •

edited

Loading

egregius313 left a comment •

edited

Loading