New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(core): don't allow arbitrary code execution when manipulating cache #8958
fix(core): don't allow arbitrary code execution when manipulating cache #8958
Conversation
Nx Cloud ReportCI is running for commit 4ab065c. 📂 Click to track the progress, see the status, the terminal output, and the build insights. Sent with 💌 from NxCloud. |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/nrwl/nx-dev/2C27XCUY2VFpTghqFDKcjnEzUwyo [Deployment for 4ab065c canceled] |
This change looks good, but CI is frozen. If you check the box to allow edits to the branch from maintainers, I can push up a commit to let CI start. Here's the docs on that: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork |
@AgentEnder The "Allow edits from maintainers" checkbox is missing, probably because the fork is made by an organization, not a user. |
Ah, yes. That happens sometimes 😅, let me see if I can figure out how to get Circle to kick off. |
Don't think it's possible, same thing happened in dequelabs/axe-core#3018, the maintainer ended up cherry-picking my commits (in order to preserve attribution) to another branch in order to get the CI to run. |
Hi @sorin-davidoi, we are unable to push to your branch due to blocked third-party maintainers. Can you try doing the following?:
git remote add upstream git@github.com:nrwl/nx.git
git fetch upstream master
git checkout fix-task-runner-cache-shell-commands #if not already on the branch
git rebase upstream/master
That should kick start the build. Otherwise, I would kindly ask you to create another PR with your personal GitHub account. |
The Node documentation for `exec` states: > Never pass unsanitized user input to this function. Any input containing shell metacharacters may be used to trigger arbitrary command execution. The `folder` variable comes directly from the `NX_CACHE_DIRECTORY` environment variable (or from `nx.json`). Careful crafting of this variable can result in NX executing arbitrary commands. This patch fixes this by using `execFile`, which does not spawn a shell.
e6b1b8d
to
4ab065c
Compare
Didn't work, closing in favor of #9329. |
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
A carefully crafted
NX_CACHE_DIRECTORY
(environment variable) can make NX execute arbitrary commands. The same holds true for thecacheDirectory
field innx.json
.Expected Behavior
NX should not execute arbitrary code embedded in the
NX_CACHE_DIRECTORY
environment variable.Fix
The Node documentation for
exec
states:The
folder
variable comes directly from theNX_CACHE_DIRECTORY
environment variable (or fromnx.json
). Careful crafting of this variable can result in NX executing arbitrary commands.This patch fixes this by using
execFile
, which does not spawn a shell.