Skip to content

RunTaskDialog: validate executable before accepting#56

Closed
sidharthify wants to merge 1 commit into
benapetr:masterfrom
sidharthify:master
Closed

RunTaskDialog: validate executable before accepting#56
sidharthify wants to merge 1 commit into
benapetr:masterfrom
sidharthify:master

Conversation

@sidharthify
Copy link
Copy Markdown

Running a non-existent program or file through "Run New Task" would silently succeed because the command is passed to /bin/sh -c, which always launches successfully regardless of whether the target exists.

Add an accept() override in RunTaskDialog that validates the executable before the dialog closes.

image

Running a non-existent program or file through "Run New Task" would
silently succeed because the command is passed to `/bin/sh -c`, which
always launches successfully regardless of whether the target exists.

Add an `accept()` override in RunTaskDialog that validates the
executable before the dialog closes.

Signed-off-by: sidharthify <wednisegit@gmail.com>
@benapetr
Copy link
Copy Markdown
Owner

Hello,

I reviewed the change and there are two major issues - first of all you aren't looking for executable, just for any existing path, so if that path provided points to a directory, or even a block device or similar nonsense, it's still accepted. That's not such a big issue given that we accepted anything earlier, but there is another much bigger issue, that dialog claims "Enter a command, script, or path to execute" and many shell commands are no longer accepted! You extract a first token (separated by space) and check if that's a path that exists. First token can be env variable, such as:

LD_LIBRARY_PATH=/usr/local/lib /usr/bin/foo

In this case you would just check if LD_LIBRARY_PATH=/usr/local/lib is a path (it's not) and rejected execution that would otherwise be OK.

However I agree with the spirit of the change, we should definitely not blindly accept any input and not even give user a feedback that is failed to run (I would probably prefer the concept where we "accept anything" since it's really hard to tell if it's gonna be executable and instead of guessing before launch I would evaluate exec return code to see if it succeeded or failed, that's much easier to implement and more bulletproof)

@benapetr
Copy link
Copy Markdown
Owner

I did this - 9f76c4f which pretty much solves the same problem, slightly differently, it drops the shell execution hack entirely and always just runs a command directly, it also changes wording of that run dialog so it no longer pretends it's a shell. Similar check whether executable exists was also implemented.

@benapetr benapetr closed this May 10, 2026
@sidharthify
Copy link
Copy Markdown
Author

Hi! Thank you for going over my code, I wrote this in a hurry and did not think about the specifics. I apologize for that.

I agree with everything and I'm glad you wrote your own fix.

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