Skip to content

Conversation

@danieljoos
Copy link

@danieljoos danieljoos commented Dec 12, 2024

These templates were rendered using text/template which is fundamentally broken as it would allow for trivial HTML injection.

Instead render using safehtml/template so that we have automatic escaping.


This is backporting the fix for CVE-2024-53257 to release-18.0-github.

Description

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

These templates were rendered using text/template which is fundamentally
broken as it would allow for trivial HTML injection.

Instead render using safehtml/template so that we have automatic
escaping.

Signed-off-by: Dirkjan Bussink <[email protected]>
Copilot AI review requested due to automatic review settings December 12, 2024 09:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 6 changed files in this pull request and generated no suggestions.

Files not reviewed (1)
  • go/vt/vtgate/debugenv.go: Evaluated as low risk
Comments skipped due to low confidence (3)

go/vt/vttablet/tabletserver/querylogz_test.go:39

  • Ensure that the test case covers scenarios where the SQL query contains potentially malicious content to verify that the safehtml/template correctly sanitizes the output.
logStats.OriginalSQL = "select name, 'inject <script>alert();</script>' from test_table limit 1000"

go/vt/vtgate/querylogz.go:25

  • Ensure that all templates previously rendered with text/template are now rendered with safehtml/template to prevent HTML injection vulnerabilities.
+	"github.com/google/safehtml/template"

go/vt/vttablet/tabletserver/querylogz.go:25

  • [nitpick] Ensure that the import alias for the safehtml/template package is consistent across the codebase for clarity.
"github.com/google/safehtml/template"

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@arthurschreiber arthurschreiber merged commit 1f2e2a7 into release-18.0-github Dec 13, 2024
200 of 206 checks passed
@danieljoos danieljoos deleted the danieljoos-backport-cve-2024-53257-fix branch December 17, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants