-
Notifications
You must be signed in to change notification settings - Fork 20
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
Remove bind method from Safemode::Scope #47
Conversation
The bind method could be used to execute arbitrary system commands by leveraging that things passed into it were getting eval'd without any escaping. This should remove the issue completely by making bind unavailable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @adamruzicka, looks sane, but I'm quite limited here, so just my two cents are:
@@ -80,6 +80,10 @@ def test_interpolated_xstr_should_raise_security | |||
assert_raise_security '"#{`ls -a`}"' | |||
end | |||
|
|||
def test_should_not_allow_access_to_bind | |||
assert_raise_security "self.bind('an arg')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though there is no longer bind
method, is this possible to misuse get_binding
directly instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how.
The original issue was that one could call bind from a template and pass arguments to it, which were not treated safely. The arguments were just evaluated as-is to set values inside the binding. As far as I understand there wasn't anything inherently unsafe about being able to retrieve the binding, apart from the way how it could be obtained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, now I have nothing to add :/
The weird thing is though that the tests didn't fail once...
Alright, let's get this in |
Hi @adamruzicka can we get this released into a new gem? |
The bind method could be used to execute arbitrary system commands by leveraging that things passed into it were getting eval'd without any escaping. This should remove the issue completely by making bind unavailable.