Skip to content

security: fix shell injection vulnerabilities in file path handling#41

Merged
dmtrKovalenko merged 1 commit into
dmtrKovalenko:mainfrom
sQVe:fix/shell-injection-vulnerabilities
Aug 4, 2025
Merged

security: fix shell injection vulnerabilities in file path handling#41
dmtrKovalenko merged 1 commit into
dmtrKovalenko:mainfrom
sQVe:fix/shell-injection-vulnerabilities

Conversation

@sQVe
Copy link
Copy Markdown
Contributor

@sQVe sQVe commented Aug 3, 2025

Summary

Fixes shell injection vulnerabilities in file path handling where malicious file names could potentially execute arbitrary commands.

What was the issue?

A few system command calls were using unescaped file paths, which could be problematic if someone created files with special shell characters in their names.

For example, a file named image.jpg; rm -rf / could cause issues when the picker tries to preview it.

What changed?

Updated 5 system command calls across 2 files to use vim.fn.shellescape() for proper path escaping:

lua/fff/file_picker/image.lua
  • Fixed file and identify commands in get_image_dimensions()
lua/fff/file_picker/preview.lua
  • Fixed tail command in read_file_tail()
  • Fixed file and xxd commands in preview_binary_file()

The fix:

-- Before
string.format('command "%s"', file_path)

-- After  
string.format('command %s', vim.fn.shellescape(file_path))

Testing

  • Verified the picker still works normally with regular files
  • Tested with files that have special characters in their names
  • Image previews and binary file handling work as expected

Replace unsafe string.format() calls with vim.fn.shellescape() to prevent
command injection when file paths contain special characters.
@dmtrKovalenko dmtrKovalenko merged commit f9ee0a2 into dmtrKovalenko:main Aug 4, 2025
6 checks passed
@dmtrKovalenko
Copy link
Copy Markdown
Owner

thank you for noting and fixing this, this is amazing!

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.

2 participants