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

remote shell execution in v1.12.2 #100

Open
ooooooo-q opened this issue Mar 2, 2022 · 4 comments
Open

remote shell execution in v1.12.2 #100

ooooooo-q opened this issue Mar 2, 2022 · 4 comments

Comments

@ooooooo-q
Copy link

I confirmed from the 038e457 commit that there are other attack methods.

# call `send` from `public_send`
ImageProcessing::Vips.apply({ send: ["system", "echo CALL_SEND" ]})

# call `method_missing`
ImageProcessing::Vips.apply({ system!: "echo CALL_SYSTEM!" })

It seems that other unexpected behavior is possible, so I think it is better to make allow list and deal with it.

@janko
Copy link
Owner

janko commented Mar 3, 2022

Thanks for the report. I don't know how I would build an allow list, especially considering the different processing backends. I will try adding a deny list for #send, #public_send, and #__send__, as well as fix #method_missing to also call #public_send instead of #send.

janko added a commit that referenced this issue Jul 24, 2024
It doesn't fully resolve the security vulnerability, and there is no
point in only partially resolving it.

See #100

This reverts commit aed5b80.
@brendon
Copy link
Contributor

brendon commented Feb 8, 2025

Did this one already get dealt with? Thought I saw something in the changes recently.

@janko
Copy link
Owner

janko commented Feb 8, 2025

There was an attempt on my side, but this issue showed there were other ways, so I reverted the non-fix.

TBH I don't consider this important to address, because inferring processing options directly from user input is a terrible idea. Even if I fixed remote shell execution, you would still allow trivial DoSing.

I believe I saw Active Storage adding a whitelist on their side, and they're the biggest dependants.

@brendon
Copy link
Contributor

brendon commented Feb 8, 2025

Sounds good :)

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

No branches or pull requests

3 participants