Go: database local source models

[egregius313]

Pull Request checklist

All query authors

• A change note is added if necessary. See the documentation in this repository.
• All new queries have appropriate .qhelp. See the documentation in this repository.
• QL tests are added if necessary. See Testing custom queries in the GitHub documentation.
• New and changed queries have correct query metadata. See the documentation in this repository.

Internal query authors only

• Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required).
• Changes are validated at scale (internal access required).
• Adding a new query? Consider also adding the query to autofix.

[github-actions]

⚠️ 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:

-    `Bun <https://bun.uptrace.dev/>`_,``github.com/uptrace/bun*``,,,63
+    `Bun <https://bun.uptrace.dev/>`_,``github.com/uptrace/bun*``,18,,63
-    `Couchbase official client(gocb) <https://github.com/couchbase/gocb>`_,"``github.com/couchbase/gocb*``, ``gopkg.in/couchbase/gocb*``",,36,16
+    `Couchbase official client(gocb) <https://github.com/couchbase/gocb>`_,"``github.com/couchbase/gocb*``, ``gopkg.in/couchbase/gocb*``",44,98,16
-    `Couchbase unofficial client <http://www.github.com/couchbase/go-couchbase>`_,``github.com/couchbaselabs/gocb*``,,18,8
+    `Couchbase unofficial client <http://www.github.com/couchbase/go-couchbase>`_,``github.com/couchbaselabs/gocb*``,22,49,8
-    `GoFrame <https://goframe.org/en/>`_,``github.com/gogf/gf*``,,,51
+    `GoFrame <https://goframe.org/en/>`_,``github.com/gogf/gf*``,12,30,51
-    `Squirrel <https://github.com/Masterminds/squirrel>`_,"``github.com/Masterminds/squirrel*``, ``github.com/lann/squirrel*``, ``gopkg.in/Masterminds/squirrel``",,,96
+    `Squirrel <https://github.com/Masterminds/squirrel>`_,"``github.com/Masterminds/squirrel*``, ``github.com/lann/squirrel*``, ``gopkg.in/Masterminds/squirrel``",84,,96
-    Totals,,494,958,1556
+    Totals,,674,1081,1556

• Changes to framework-coverage-go.csv:

- github.com/Masterminds/squirrel,32,,,,,,,,,,,,,,32,,,,,,,,,,,,
+ github.com/Masterminds/squirrel,32,28,,,,,,,,,,,,,32,,,,,,28,,,,,,
- github.com/couchbase/gocb,8,,18,,,,,8,,,,,,,,,,,,,,,,,,18,
+ github.com/couchbase/gocb,8,22,49,,,,,8,,,,,,,,,,,,,22,,,,,49,
- github.com/couchbaselabs/gocb,8,,18,,,,,8,,,,,,,,,,,,,,,,,,18,
+ github.com/couchbaselabs/gocb,8,22,49,,,,,8,,,,,,,,,,,,,22,,,,,49,
- github.com/gogf/gf/database/gdb,51,,,,,,,,,,,,,,51,,,,,,,,,,,,
+ github.com/gogf/gf/database/gdb,51,12,30,,,,,,,,,,,,51,,,,,,12,,,,,30,
- github.com/lann/squirrel,32,,,,,,,,,,,,,,32,,,,,,,,,,,,
+ github.com/lann/squirrel,32,28,,,,,,,,,,,,,32,,,,,,28,,,,,,
- github.com/uptrace/bun,63,,,,,,,,,,,,,,63,,,,,,,,,,,,
+ github.com/uptrace/bun,63,18,,,,,,,,,,,,,63,,,,,,18,,,,,,
- gopkg.in/Masterminds/squirrel,32,,,,,,,,,,,,,,32,,,,,,,,,,,,
+ gopkg.in/Masterminds/squirrel,32,28,,,,,,,,,,,,,32,,,,,,28,,,,,,
- gopkg.in/couchbase/gocb,8,,18,,,,,8,,,,,,,,,,,,,,,,,,18,
+ gopkg.in/couchbase/gocb,8,22,49,,,,,8,,,,,,,,,,,,,22,,,,,49,

[owen-mc]

Two tests need to have their results updated. go/ql/test/query-tests/Security/CWE-079/StoredXss.qlref also needs to have a post-processing query added for pretty-printing models, as in this PR.

I haven't had time to look at the models themselves yet. Are these new models, or are you converting models from QL? If the latter, should we also delete the QL models?

[egregius313]

Are these new models, or are you converting models from QL? If the latter, should we also delete the QL models?

Yes these are primarily new models. There weren't many existing database models as far as I could tell.

The one package where I'm unsure about the existing models vs the new ones are the beego-orm models. The discrepancy seems to be from API changes in v2

[owen-mc]

I've only reviewed half of the new models so far (stdlib, beego, couchbase).

I note that we do have some existing database read models - see the source definition for the stored xss query. We should convert them to MaD, deprecate the QL classes where they are only used for that, and use the database MaD sources for that query (ignoring what the active threat model is, I think).

Extremely minor stylistic comment: within one .model.yml file I prefer to put sources first, sinks second and summaries third. This is because there are generally the fewest sources, the second fewest sinks and the most summaries. It makes it slightly easier to read and see at a glance which extensible predicates are being extended. When summaries go first, it is easy to miss that other predicates are extended below.

[owen-mc]

• Please test for all sources defined in each package.
• That will probably make the file a bit longer - I would split this up into a separate source file for each package, so it is easier to find particular tests.

[owen-mc]

I think it would make more sense to model Cluster.Query and Scope.Query (which are the only way of getting a QueryResult) as the sources and model the two methods above as summary models. There is a nice symmetry that the same function is an sql-injection sink for one argument and a database source for the return value.

Also, add summary models for QueryResult.Raw and QueryResultRaw.NextBytes.

[owen-mc]

I also think that Cluster.AnalyticsQuery and Scope.AnalyticsQuery should be sources, and all the summary models for QueryResult should be duplicated for AnalyticsResult.

[owen-mc]

Same as above, I think TransactionAttemptContext.Query should be the source and these should be summary models. (It should probably be an sql-injection sink as well - I've noted this down somewhere else, no need to do it in this PR.)

[owen-mc]

Presumably we should model LookupInAllReplicas and LookupInAnyReplica as sources, and a summary model for LookupInAllReplicasResult.Next.

[owen-mc]

We also need a summary model for ScanResultItem.Content

[owen-mc]

I've done another four libraries. jmoiron/sqlx was such hard going!

[owen-mc]

Also want models for methods Scan, ScanList, Select, Struct and Structs on Model.

[owen-mc]

Also, most of these methods on Model return a Model, which we aren't modelling at the moment. It would mean a huge number of models though. But without that, the examples they give, like db.Model("user").Where("id", 1).Scan(&user) , won't get the taint tracked through it properly. I'm not sure what to do here, to be honest.

[owen-mc]

Also needs models for QueryContextWith, QueryWith, QueryRowContextWith, QueryRowWith, Row.Scan (it seems to override the inherited Scan method from embedding RowScanner), *Builder.Query (different method signature to QueryRower.Query, unfortunately), *Builder.QueryContext, *Builder.QueryRow, *Builder.QueryRowContext, *Builder.Scan, *Builder.ScanContext. (Do we care about all the Builders? Delete? I guess safest to model them all...)

[owen-mc]

I've finished going through all the libraries. Modeling is such hard work! Thanks for doing all this - it must have taken a long time.

[owen-mc]

If I remember correctly, this model won't work because dataflow out of a variadic parameter doesn't work currently. It can be modeled in QL.

[owen-mc]

The last example here shows that we need to model Exec as well (in QL). And this part of the docs.

[owen-mc]

These docs make me think we need source models for DB.Query* and summary models for DB.ScanRow and DB.ScanRows.

[owen-mc]

Superseded by #19203.