Skip to content
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

Add security warning to README and lib.rs doc #40

Closed
Marcono1234 opened this issue Oct 11, 2023 · 5 comments · Fixed by #41
Closed

Add security warning to README and lib.rs doc #40

Marcono1234 opened this issue Oct 11, 2023 · 5 comments · Fixed by #41

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Oct 11, 2023

Due to tinyfiledialogs trying to support many operating systems and environments, and due to how it is implemented (generating command line strings, generating Python code, ...) it seems to be inherently prone to command injection vulnerabilities, see for example #19.

What do you therefore think about adding a security warning to the README of this project to the documentation of the lib.rs file?

For example something like this:

...

Security Warning

tinyfiledialogs should only be used with trusted input. Using it with untrusted input for example as dialog title or message can in the worst case lead to execution of arbitrary commands.

My intention here is not fearmongering, but instead to indicate to users for which use cases tinyfiledialogs is suitable, and for which it might not.

(CC @vareille; is this information correct? Should there maybe also be a warning in the README of the upstream project?)

@jdm
Copy link
Owner

jdm commented Oct 11, 2023

I would accept a pull request that added a warning.

@vareille
Copy link

vareille commented Oct 12, 2023

Hi tinyfiledialogs rejects ' " ` and generate a warning accordingly. You should update to the latest version 3.15 on sourceforge.

edit: I have just also added a warning at the beginning of all the files in v3.15.1

@Marcono1234
Copy link
Contributor Author

Marcono1234 commented Oct 12, 2023

Have created #41

Hi tinyfiledialogs rejects ' " ` and generate a warning accordingly.

Ah you are right. But the function quoteDetectedW in the upstream version only checks for " and '; is that intended or an oversight?

But I am not sure if there might be other characters which can also cause problems. Maybe also related is native-toolkit/libtinyfiledialogs#5 (not sure if that is still relevant though).

edit: I have just also added a warning at the beginning of all the files in v3.15.1

Thanks!

@vareille
Copy link

vareille commented Oct 13, 2023

quoteDetectedW is only on windows and ` has only an effect in Unix shells.
The issue native-toolkit/libtinyfiledialogs#5 with < is a zenity behavior (bug ?). I think they know about it and don't seem to care.

@jdm jdm closed this as completed in #41 Oct 13, 2023
@Marcono1234
Copy link
Contributor Author

Also @jdm and @vareille, sorry if this GitHub issue made you feel pressured to add such a security notice. That was not my intention.

To me it seemed tinyfiledialogs hadn't been designed to handle arbitrary input and that as seen by the other issues it does not handle some input very well, but that this just hadn't been explicitly mentioned in the README before.
And therefore directly providing potentially untrusted input as dialog message isn't an intended or supported use case of tinyfiledialogs.

If my assessment was incorrect or if you think the security warning added by my pull request is misleading or too alarmist, feel free to adjust it or let me know how I should adjust it.

My intention wasn't to harm the reputation of the Rust bindings project here or tinyfiledialogs in general.

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 a pull request may close this issue.

3 participants