Conversation
Plugin sources, refs, and commits from quartz.config.yaml and quartz.lock.json were interpolated into shell command strings. Quoting the URL is not sufficient — $(...) and backticks still expand inside double quotes — and refs, commits, and the --branch argument were not quoted at all, so a crafted lockfile entry ran arbitrary commands on the machine doing the build. Reproduction on v5 before this change, with a lockfile ref of "$(touch /tmp/quartz-pwned)": `quartz plugin install` reports the clone as failed and creates /tmp/quartz-pwned anyway. Every git invocation now goes through execFile with an argv array, so no shell is involved. Values that still reach git as arguments are validated: commits must be hex, refs must look like refnames, and a repository may not begin with "-". Clones also pass "--" before the repository so a URL cannot be parsed as a flag (--upload-pack=<cmd>). `npm install <pkg>` for npm-sourced plugins gets the same treatment. Verified: prettier clean, 109/109 tests, and plugin add/install/update/ check all still work against a real repository.
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
This branch was successfully deployed
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
quartz/cli/plugin-git-handlers.jsbuilds shell command strings from values that come out ofquartz.config.yamlandquartz.lock.json:The repository URL is double-quoted, but
$(...)and backticks still expand inside double quotes, and the ref, commit, and--branchargument are not quoted at all. A crafted entry therefore runs arbitrary commands on whatever machine performs the install — a contributor's laptop, or CI.Reproduction (on
v5, before this change)Install any plugin, then set a ref in
quartz.lock.json:The clone reports failure and the injected command runs anyway, so nothing in the output signals what happened.
I hit this while upgrading a fork to v5; happy to adjust the approach if you'd prefer it shaped differently.
Fix
execFilewith an argv array, so no shell is involved/^[0-9a-f]{7,40}$/i), refs must look like refnames and may not contain.., and a repository may not begin with---before<repository>, so a URL can never be parsed as a flag such as--upload-pack=<cmd>npm install <pkg>for npm-sourced plugins gets the same argv treatmentWith the patch, the reproduction above fails the clone and creates no file.
Testing
npm test— 109/109 passnpx prettier --check— cleanplugin add,plugin install,plugin update,plugin checkall still clone, build, and report correctly