gnunn1 / tilix

A tiling terminal emulator for Linux using GTK+ 3
https://gnunn1.github.io/tilix-web
Mozilla Public License 2.0
5.36k stars 293 forks source link

fix(security): avoid shell injection in open-tilix plugin #2155

Closed taoky closed 8 months ago

taoky commented 1 year ago

I first reported this issue in https://bugs.archlinux.org/task/77698.

Currently tilix's open-tilix plugin for nautilus uses subprocess.call() with shell=True. However it fails to sanitize input data (filename) correctly, thus causing possible shell injection when filename contains " or `, etc.

This PR tries to solve this issue by using subprocess.Popen() and avoiding invoking shell:

ximion commented 1 year ago

Can you adjust the patch so it works on top of the Nautilus plugin updates in master? Thanks for looking into this and creating this patch!

taoky commented 1 year ago

Can you adjust the patch so it works on top of the Nautilus plugin updates in master? Thanks for looking into this and creating this patch!

OK, the conflict has been resolved.

ximion commented 8 months ago

Are you sure? It appears there's still a conflict and this PR can't be merged right now...

taoky commented 8 months ago

Are you sure? It appears there's still a conflict and this PR can't be merged right now...

This PR page shows "This branch has no conflicts with the base branch". I don't know where the "conflict" is.

image

And the "incomplete" label is incorrect - this PR is a complete work and is just waiting for reviewing & merging.

ximion commented 8 months ago

Ah, the problem is that you have a merge commit in thie PR, so rebase does not work. But I can squash down the PR and merge it that way.

ximion commented 8 months ago

Thank you for the patch! :-)