codeql
codeql copied to clipboard
Go: `template/text.Template` execution methods: support reading arbitrary content
NB. This requires adding a hook to shared dataflow; this will need adding (as an empty stub) to other languages.
: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``",52,605,104
+ `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``",52,603,104
- Totals,,459,943,1532
+ Totals,,459,941,1532
- Changes to framework-coverage-go.csv:
- text/template,,,6,,,,,,,,,,,,,,,,,,,,,,,6,
+ text/template,,,4,,,,,,,,,,,,,,,,,,,,,,,4,
@smowton What is the status of this? Is it waiting for review, or are you still working on it?
@owen-mc thanks to my currently blurry work situation I found time to fix it up. Currently running QA; please do review.
QA results: new TPs from around 20 repos. Found and fixed an FP based on defining an interface that could be text/template.Template or could be html/template.Template (the latter is sanitizing, and in practice text/template is not used).
Significant slowdowns in two, which so far as I can see are simply caused by significantly more flow being possible when text-template instantiation is able to pull taint down from fields to top level. I propose to go ahead on grounds that the vast majority of repositories suffer no consequences, and that is real flow (though we could iterate to try to find heuristics to prevent excessively-expensive exploration if necessary).
I rebased this on latest main and reran QA: found one large performance outlier, yaklang/yaklang. Tested whether the dataflow performance improvement I discovered before Christmas works there too; started a DCA experiment to check and reported more fully at https://github.com/github/codeql/pull/18355
Performance issue with yaklang resolved by https://github.com/github/codeql/pull/18515