codeql icon indicating copy to clipboard operation
codeql copied to clipboard

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

Open owen-mc opened this issue 1 year ago • 2 comments

These were not converted when most of the rest of the models were converted in https://github.com/github/codeql/pull/12750 because at the time flow didn't work through variadic parameters when using models-as-data. That was fixed in https://github.com/github/codeql/pull/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 avatar May 25 '24 20:05 owen-mc

:warning: 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,

github-actions[bot] avatar May 25 '24 20:05 github-actions[bot]

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 avatar May 28 '24 19:05 owen-mc