Doubiiu / ToonCrafter

[SIGGRAPH Asia 2024, Journal Track] ToonCrafter: Generative Cartoon Interpolation
https://doubiiu.github.io/projects/ToonCrafter/
Apache License 2.0
5.4k stars 452 forks source link

Security Concerns with a Windows Fork of This Project #13

Open jjhaggar opened 6 months ago

jjhaggar commented 6 months ago

Hi,

I'm looking into using your project and I noticed a Windows-specific fork ( https://github.com/sdbds/ToonCrafter-for-windows ) that alters the installation process by setting the PowerShell execution policy to Unrestricted. This raises some security red flags for me.

I’d like to ask:

Your insights would really help me and other potential users decide whether to use this fork or wait for an official update.

Thanks for your work on this project and looking forward to your thoughts!

UberStorm commented 6 months ago

It doesn't work anyways :(

iloveoss commented 6 months ago

jjhaggar There are concerns but what security concerns are you worried of? That page asks "YOU" the "USER" to enable "Set-ExecutionPolicy Unrestricted". Given this is not one-click install .exe file, I think people who are going to type in all those codes (or paste) them, will have some understanding of what they are doing.

I mean just check the script bro "https://github.com/sdbds/ToonCrafter-for-windows/blob/main/install.ps1" its doing nothing except for creating a virtual environment and downing the necessary models for you.

jjhaggar commented 6 months ago

Hi @iloveoss, thanks for engaging in this discussion! ^_^/

While some users might handle basic commands, many follow instructions without fully understanding the implications. Setting the execution policy to Unrestricted poses risks as it allows any script to run unchecked (not just the installation one), which could be exploited if any subsequent script contains vulnerabilities or malicious code.

Also, the installation process doesn't include steps to revert to a safer execution policy, leaving users potentially unprotected. With the use of Python scripts when the Gradio app is launched, it’s crucial to ensure no PowerShell scripts could be triggered unknowingly from Python, as cross-technology scripting can introduce significant security challenges. I haven't reviewed the full repo, and I'm quite sure not many people will do that before installing.

Additionally, using .ckpt models instead of .safetensors introduces potential vulnerabilities, a concern that extends to the original repository as well. Perhaps we could consider safer practices here too?

I realize I might be overreacting, but I believe it's better to be safe than sorry when it comes to security practices. ^_^U

UberStorm commented 6 months ago

You can use https://github.com/kijai/ComfyUI-DynamiCrafterWrapper/ it uses safetensors model for ToonCrafter and works very good on my 4070ti

sdbds commented 6 months ago

Hi @iloveoss, thanks for engaging in this discussion! ^_^/

While some users might handle basic commands, many follow instructions without fully understanding the implications. Setting the execution policy to Unrestricted poses risks as it allows any script to run unchecked (not just the installation one), which could be exploited if any subsequent script contains vulnerabilities or malicious code.

Also, the installation process doesn't include steps to revert to a safer execution policy, leaving users potentially unprotected. With the use of Python scripts when the Gradio app is launched, it’s crucial to ensure no PowerShell scripts could be triggered unknowingly from Python, as cross-technology scripting can introduce significant security challenges. I haven't reviewed the full repo, and I'm quite sure not many people will do that before installing.

Additionally, using .ckpt models instead of .safetensors introduces potential vulnerabilities, a concern that extends to the original repository as well. Perhaps we could consider safer practices here too?

I realize I might be overreacting, but I believe it's better to be safe than sorry when it comes to security practices. ^_^U

Hi, this is a branch I made to allow no code experience people to quickly install projects on windows. In fact the script run permissions are as secure as exe, as running non-system scripts is prohibited by default on windows.

If you are concerned about security, you can either install the contents of this project step by step, or open ps1 in notepad and then run the script's commands one by one.

jjhaggar commented 6 months ago

Hi @sdbds, thanks a lot for your response! :D

So, if I understood correctly, the Unrestricted execution policy is only needed for the install.ps1 script, and there is an alternative that doesn’t require changing the policy—executing the steps inside that script manually, right?

If that's the case, I’d suggest adding this information to your README.md. It could encourage more people to try using it, knowing they have safer installation options.

Thanks for your efforts in making this more accessible!

Also, thank you for pointing up that ComfyUI node, @UberStorm! :D

sdbds commented 6 months ago

Hi @sdbds, thanks a lot for your response! :D

So, if I understood correctly, the Unrestricted execution policy is only needed for the install.ps1 script, and there is an alternative that doesn’t require changing the policy—executing the steps inside that script manually, right?

If that's the case, I’d suggest adding this information to your README.md. It could encourage more people to try using it, knowing they have safer installation options.

Thanks for your efforts in making this more accessible!

Also, thank you for pointing up that ComfyUI node, @UberStorm! :D

To be precise this is the default setting implemented in the windows venv environment.

jjhaggar commented 6 months ago

To be precise this is the default setting implemented in the windows venv environment.

Thanks for the response. I'm still a bit confused, though (pretty confused to be honest ^_^U). Could you explain what you mean by 'default setting in the windows venv environment'? Does that mean that we have to set the policy to Unrestricted each time we want to launch the ´run_gui´ script? Also, does changing this setting affect the entire system or just the virtual environment? How can I check this myself? Thanks for your help! :)