Skip to content

fix: quote arguments for passing to auto completion server #20

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

Merged
merged 1 commit into from
Apr 5, 2025

Conversation

kazupon
Copy link
Contributor

@kazupon kazupon commented Apr 5, 2025

I realized there was code in the zsh completion generation code that caused vulnerabilities.

tab/src/zsh.ts

Line 61 in d5659d5

out=$(eval \${requestComp} 2>/dev/null)

Here, the requestComp variable contains the words that the user entered on the command line (${words[2,-1]}).
The eval command reinterprets this string as a shell command and executes it.

issue point

If the user enters a string containing shell metacharacters (e.g. ;, &, |, $(), etc.) as a command line argument, eval will interpret these metacharacters as command delimiters or subcommand execution with special meaning.
This may cause unintended arbitrary commands to be executed.

example

Suppose the user types the following (name is mycli, and exec is mycli):

mycli some-command '; rm -rf ~ #'

In this case, requestComp would be a string like “mycli complete -- some-command '; rm -rf ~ #”. If eval executes this, the following commands may be executed in order.

As you can see, because the user input is not properly escaped or quoted, there is a risk that arbitrary commands could be injected by using eval.

tab case may be a rare case, but if it is input in some way, it could cause comm

Copy link

changeset-bot bot commented Apr 5, 2025

⚠️ No Changeset found

Latest commit: c416a6b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

pkg-pr-new bot commented Apr 5, 2025

Open in StackBlitz

npm i https://pkg.pr.new/bombshell-dev/tab@20

commit: c416a6b

Copy link
Collaborator

@43081j 43081j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me 👍

good catch. we should also have some tests around this eventually, I'll create a separate issue for that

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @kazupon, LGTM 🫡

@43081j 43081j merged commit 820a3d6 into bombshell-dev:main Apr 5, 2025
7 checks passed
@kazupon kazupon deleted the fix/quote-args branch April 5, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants