Validate release-feed version and verify download checksums (HackerOne #3764025) - #45
Merged
Merged
Conversation
…a root command-injection path get_current_simplerisk_version() fetched the version string from releases.xml with no validation, and set_up_simplerisk() interpolated it unescaped into wget/tar commands run through exec_cmd -> bash -c. A compromised feed (or its GitHub fallback, or a trusted-CA MITM) could embed shell metacharacters in the version attribute and get root code execution on every install/upgrade, since the script enforces EUID==0. Two independent layers of defense, since either alone would leave a gap: - get_current_simplerisk_version() now rejects anything that doesn't match releases.xml's actual version format (YYYYMMDD-NNN) before the value is used anywhere. - set_up_simplerisk()'s wget/tar calls are converted from exec_cmd (bash -c string interpolation) to run_cmd (array-safe, argv passed directly), so the version string is never re-evaluated by a shell at all, regardless of the regex above. Also verifies the downloaded bundle and database.sql against the bundle_sha256/database_schema sha256 releases.xml already publishes for each release (verify_sha256()), aborting on any mismatch. This closes a second gap the regex/run_cmd changes don't: an artifact host (S3, GitHub raw) or the download itself being tampered with independently of releases.xml. Verified: the exact malicious version string from the report is now rejected before reaching any shell context; the real releases.xml's version and both checksums parse and verify correctly against a real download; a tampered downloaded file is correctly caught and rejected; and a full install -> verify -> uninstall -> verify cycle against a fresh Ubuntu 22.04 container shows no regression (the fixed code path is OS-agnostic, shared identically by every supported OS family). Reported via HackerOne #3764025. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
get_current_simplerisk_version()fetched the version string fromreleases.xmlwith no validation, andset_up_simplerisk()interpolated it unescaped intowget/tarcommands run throughexec_cmd→bash -c. A compromised feed (or its GitHub fallback, or a trusted-CA MITM) could embed shell metacharacters in the version attribute and get root code execution on every install/upgrade, since the script enforcesEUID==0.get_current_simplerisk_version()now rejects anything that doesn't matchreleases.xml's real version format (YYYYMMDD-NNN) before the value is used anywhere.set_up_simplerisk()'swget/tarcalls are converted fromexec_cmd(bash -cstring interpolation) torun_cmd(array-safe, argv passed directly), so the version string is never re-evaluated by a shell at all, regardless of the regex above.database.sqlagainst thebundle_sha256/database_schemasha256thatreleases.xmlalready publishes per release (newverify_sha256()), aborting on mismatch. This closes a second gap the two changes above don't: an artifact host (S3, GitHub raw) or the download itself being tampered with independently ofreleases.xml.Reported via HackerOne #3764025.
Test plan
releases.xmland confirmed version + both checksums parse correctly.verify_sha256catches it and aborts.