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

Prefer File.open instead of Kernel#open #4529

Merged
merged 1 commit into from Apr 13, 2021

Conversation

mame
Copy link
Contributor

@mame mame commented Apr 13, 2021

... because Kernel#open may allow unintentional command injection.
At present, I don't know how to exploit these usages of Kernel#open, but
I'm afraid if they may be used with untrusted user input accidentally in
future. As far as I see, there is no reason to use Kernel#open.

... because Kernel#open may allow unintentional command injection.
At present, I don't know how to exploit these usages of Kernel#open, but
I'm afraid if they may be used with untrusted user input accidentally in
future. As far as I see, there is no reason to use Kernel#open.
@hsbt hsbt merged commit 4795180 into rubygems:master Apr 13, 2021
@hsbt
Copy link
Member

hsbt commented Apr 13, 2021

👍

mame added a commit to mame/rubygems that referenced this pull request Apr 13, 2021
Ditto to IO.readlines. IO.read and IO.readlines may run a command.

Related: rubygems#4529
@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Apr 13, 2021

@hsbt I don't think this is a security fix, since I don't think it can really be exploited at the moment. I think it's better to tag it as rubgygems: enhancement since it's just a preventive measure in case it somehow became exploitable in the future. I already used that same criteria to tag #4506 as an enhancement. Same thing would also apply to #4530.

@hsbt
Copy link
Member

hsbt commented Apr 14, 2021

I have no strong opinion to remain that. I replaced your suggestion.

@deivid-rodriguez
Copy link
Member

Alright @hsbt! It just felt to me that tagging this as a security fix would essentially imply that everyone using Kernel#open has a security vulnerability, which didn't seem like a good thing to imply.

deivid-rodriguez pushed a commit that referenced this pull request May 3, 2021
…-open

Prefer File.open instead of Kernel#open

(cherry picked from commit 4795180)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants