fix(security): sanitize extractor-emitted package identifiers in workflow-parsed log sinks#2928
Open
adilburaksen wants to merge 1 commit into
Open
Conversation
Several Info/Warn log sites emit package name, version and ecosystem derived from lockfile/SBOM contents to stdout without applying output.SanitizeForWorkflowCommand. On the pull_request-triggered google/osv-scanner-action path an attacker controls those values, and a package name may contain \r/\n (e.g. a package-lock.json path key or a CycloneDX component name), letting them inject ::error::, ::add-mask::, ::stop-commands:: and other GitHub Actions workflow commands into the runner's stdout. This mirrors the existing guard on pkg/osvscanner/scan.go:158 and extends it to the remaining package-identity sinks: - pkg/osvscanner/filter.go:48 (short-commit skip warning) — reachable with no config via a git dependency whose commit hash is < 40 chars - pkg/osvscanner/filter.go:104 (ignored-package notice) - pkg/osvscanner/vulnerability_result.go:110,113 (license override notices) SanitizeForWorkflowCommand is an identity function on strings that contain no \r or \n, so there is no behaviour change for typical package metadata.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2925 (cc @BiswajeetRay7, who requested a separate scoped PR for these sinks in #2925 (comment)).
Scope
#2925 sanitizes the filesystem-walk path sinks. This PR covers the complementary extractor-emitted package-identity sinks — package name/version/ecosystem read from lockfile or SBOM contents — which reach stdout unsanitized through the same
cmdlogger→Handler.Handlepath (record.Messageis written verbatim; there is no global guard). A package name can legally contain\r/\n(e.g. apackage-lock.jsonpackagespath key, or a CycloneDX component), so this is a strictly wider primitive than the path sinks: it needs only crafted lockfile contents, no unusual filenames.Sinks guarded (same
output.SanitizeForWorkflowCommandaspkg/osvscanner/scan.go:158)pkg/osvscanner/filter.go:48— short-commit skip warning (imodels.Name). Reachable with no config via a git dependency whose commit hash is< 40chars.pkg/osvscanner/filter.go:104— ignored-package notice (pkgString= ecosystem/name/version, plusreason).pkg/osvscanner/vulnerability_result.go:110,113— license-override notices.Verification
package-lock.jsonemits::error::…at the start of its own stdout line (workflow-command injection). After: the bytes are URL-encoded (…%0A::error::…) and stay on one line.go test ./pkg/osvscanner/passes with no snapshot changes (SanitizeForWorkflowCommandis identity on strings without\r/\n).Related VRP report: https://issuetracker.google.com/issues/533784193 (same threat model). Supersedes #2927 (which I opened before seeing the contributing guide; reopening as a scoped follow-up per @BiswajeetRay7's request). Happy to instead fold this into #2925 if the review team prefers a single commit.