Skip to content
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.

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

Go: Convert models for variadic functions to use models-as-data #16592

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

owen-mc
Copy link
Contributor

@owen-mc owen-mc commented May 25, 2024

These were not converted when most of the rest of the models were converted in #12750 because at the time flow didn't work through variadic parameters when using models-as-data. That was fixed in #11732.

When a class existed just function modeling and it was public instead of private I have deprecated it and removed the inheritance from TaintTracking::FunctionModel.

@owen-mc owen-mc requested a review from a team as a code owner May 25, 2024 20:44
Copy link
Contributor

github-actions bot commented May 25, 2024

⚠️ 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:
-    `Standard library <https://pkg.go.dev/std>`_,"````, ``archive/*``, ``bufio``, ``bytes``, ``cmp``, ``compress/*``, ``container/*``, ``context``, ``crypto``, ``crypto/*``, ``database/*``, ``debug/*``, ``embed``, ``encoding``, ``encoding/*``, ``errors``, ``expvar``, ``flag``, ``fmt``, ``go/*``, ``hash``, ``hash/*``, ``html``, ``html/*``, ``image``, ``image/*``, ``index/*``, ``io``, ``io/*``, ``log``, ``log/*``, ``maps``, ``math``, ``math/*``, ``mime``, ``mime/*``, ``net``, ``net/*``, ``os``, ``os/*``, ``path``, ``path/*``, ``plugin``, ``reflect``, ``reflect/*``, ``regexp``, ``regexp/*``, ``slices``, ``sort``, ``strconv``, ``strings``, ``sync``, ``sync/*``, ``syscall``, ``syscall/*``, ``testing``, ``testing/*``, ``text/*``, ``time``, ``time/*``, ``unicode``, ``unicode/*``, ``unsafe``",8,581,
+    `Standard library <https://pkg.go.dev/std>`_,"````, ``archive/*``, ``bufio``, ``bytes``, ``cmp``, ``compress/*``, ``container/*``, ``context``, ``crypto``, ``crypto/*``, ``database/*``, ``debug/*``, ``embed``, ``encoding``, ``encoding/*``, ``errors``, ``expvar``, ``flag``, ``fmt``, ``go/*``, ``hash``, ``hash/*``, ``html``, ``html/*``, ``image``, ``image/*``, ``index/*``, ``io``, ``io/*``, ``log``, ``log/*``, ``maps``, ``math``, ``math/*``, ``mime``, ``mime/*``, ``net``, ``net/*``, ``os``, ``os/*``, ``path``, ``path/*``, ``plugin``, ``reflect``, ``reflect/*``, ``regexp``, ``regexp/*``, ``slices``, ``sort``, ``strconv``, ``strings``, ``sync``, ``sync/*``, ``syscall``, ``syscall/*``, ``testing``, ``testing/*``, ``text/*``, ``time``, ``time/*``, ``unicode``, ``unicode/*``, ``unsafe``",8,626,
-    `beego <https://beego.me/>`_,"``github.com/astaxie/beego*``, ``github.com/beego/beego*``",,42,
+    `beego <https://beego.me/>`_,"``github.com/astaxie/beego*``, ``github.com/beego/beego*``",,44,
-    `zap <https://go.uber.org/zap>`_,``go.uber.org/zap*``,,11,
+    `zap <https://go.uber.org/zap>`_,``go.uber.org/zap*``,,12,
-    Totals,,20,874,24
+    Totals,,20,922,24
  • Changes to framework-coverage-go.csv:
- database/sql,,,7,,,,7,
+ database/sql,,,9,,,,9,
- errors,,,3,,,,3,
+ errors,,,4,,,,4,
- fmt,,,16,,,,16,
+ fmt,,,28,,,,28,
- github.com/astaxie/beego/utils,,,13,,,,13,
+ github.com/astaxie/beego/utils,,,14,,,,14,
- github.com/beego/beego/core/utils,,,13,,,,13,
+ github.com/beego/beego/core/utils,,,14,,,,14,
- go.uber.org/zap,,,11,,,,11,
+ go.uber.org/zap,,,12,,,,12,
- html/template,,,6,,,,6,
+ html/template,,,9,,,,9,
- io,,,19,,,,19,
+ io,,,20,,,,20,
- log,,,3,,,,3,
+ log,,,15,,,,15,
- net/textproto,,,19,,,,19,
+ net/textproto,,,21,,,,21,
- net/url,,,23,,,,23,
+ net/url,,,27,,,,27,
- path,,,5,,,,5,
+ path,,,6,,,,6,
- path/filepath,,,13,,,,13,
+ path/filepath,,,14,,,,14,
- reflect,,,37,,,,37,
+ reflect,,,39,,,,39,
- strings,,,34,,,,34,
+ strings,,,35,,,,35,
- text/template,,,6,,,,6,
+ text/template,,,9,,,,9,

@owen-mc owen-mc marked this pull request as draft May 26, 2024 13:52
@owen-mc
Copy link
Contributor Author

owen-mc commented May 28, 2024

Converting these models to models-as-data has highlighted a few bugs. This PR shouldn't be merged until they are fixed, so I've converted it to a draft.

  • There currently isn't any flow out through an implicit varargs parameter. Stdlib functions like fmt.SScan require this functionality.
  • Some barrier guard definitions use localTaintStep to detect taint flow, but it doesn't work flow through an implicit varargs slice.

@owen-mc owen-mc force-pushed the go/mad-for-variadic-functions branch from b3726d7 to 54817d6 Compare June 4, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant