pkgdb: recover from go-rpmdb panics on malformed rpm headers - #5
Open
cwayne18 wants to merge 1 commit into
Open
Conversation
Scanning an image whose rpm database carries a malformed header crashed the whole run instead of failing the scan. go-rpmdb v0.1.1 imports a header region as peList[1:ril] (entry.go:170) without checking ril >= 1, so a header whose region trailer yields a zero index-length panics with "slice bounds out of range [1:0]". rancher/rancher:v2.15.0 is one such image; the panic propagated from ListPackages up through ospkg and killed the process. The parse path has been unguarded since the rpm backend first shipped, and v0.1.1 is the latest go-rpmdb release, so there is no upstream fix to pull in. Wrap ListPackages and InstalledFileNames in a recover that turns a parser panic into an error. A database vexscan cannot parse is already treated as a scan failure by pkgdb.Read, so this keeps that contract rather than crashing. The regression test builds a real SQLite rpm database holding the smallest header that reaches the panicking slice and asserts Read returns an error. Without the recover the same test reproduces the original crash. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents vexscan from crashing when go-rpmdb panics while parsing malformed RPM headers, converting those panics into normal scan errors so a single bad RPM database cannot terminate the whole run.
Changes:
- Wrap
go-rpmdbcalls (ListPackages,InstalledFileNames) withrecover()helpers that return errors instead of propagating panics. - Add a regression test that constructs a minimal SQLite RPM DB containing a header blob known to trigger the upstream panic and asserts
RPM.Readreturns an error.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/pkgdb/rpm.go |
Adds panic-to-error wrappers for go-rpmdb parsing paths and uses them from RPM.Read. |
internal/pkgdb/rpm_panic_test.go |
Introduces a regression test that reproduces the upstream panic via a crafted RPM header stored in a real SQLite rpmdb. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Problem
Scanning an image whose rpm database carries a malformed header crashes the whole run instead of failing the scan. Reported against
rancher/rancher:v2.15.0:go-rpmdbv0.1.1 imports a header region aspeList[1:ril](entry.go:170) without checkingril >= 1. A header whose region trailer yields a zero index-length produces the slice[1:0]and panics. The panic propagates fromListPackagesup throughospkgand kills the process.Root cause / history
This is not a vexscan regression. The parse path has been unguarded since the rpm backend first shipped (
8035943, present in every release from v0.1.0), andgo-rpmdbhas been pinned at v0.1.1 the entire time. The crash is data-triggered: only a header withril == 0hits it, so a clean database never panics. v0.1.1 is the latest go-rpmdb tag and upstream has no fix for this slice, so there is nothing to pull in.Fix
Wrap
ListPackagesandInstalledFileNamesin arecoverthat turns a parser panic into an error.pkgdb.Readalready treats an unparseable-but-present database as a scan error, so this keeps that contract instead of crashing the run.Test
TestRPMSurvivesAPanickingHeaderbuilds a real SQLite rpm database (using the modernc driverrpm.goalready blank-imports — no new dependency) whose singlePackagesrow is the smallest header that reaches the panicking slice, and assertsReadreturns an error. Removing the recover makes the same test reproduce the original crash, so it is a faithful guard.go build ./...,go vet ./internal/pkgdb/, and the fullgo test ./...all pass, including the-tags norpmbuild.