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

Should not concat everything into a string to make the cmd #27

Open
darkhelmet opened this issue Dec 20, 2011 · 8 comments
Open

Should not concat everything into a string to make the cmd #27

darkhelmet opened this issue Dec 20, 2011 · 8 comments

Comments

@darkhelmet
Copy link

Big security hole and doesn't work when any filename has spaces in it.

@tcocca
Copy link
Owner

tcocca commented Dec 20, 2011

Daniel, do you have any suggestions here? How can't we call the command if we are not creating a string of the command?

Thanks for the interest in the project.

@darkhelmet
Copy link
Author

Check out the docs for popen3 http://www.ruby-doc.org/stdlib-1.8.7/libdoc/open3/rdoc/Open3.html

It works like system in that it can take variable args which it properly passes. It's the easiest way to safely pass args to a process like that. I've already got a branch mostly working, but all your documentation regarding return types (holy shit, nice docs) and what not isn't applicable anymore (haven't gotten to updating that).

@tcocca
Copy link
Owner

tcocca commented Dec 20, 2011

Cool, I'll check it out. I'd love to see the progress you have made here. Eagerly awaiting a pull request. I think this fork commit: roend83@702983e might fix the error when the filename has spaces in it.

What is the deal with the return types no longer working correctly? If you are passing args can you no longer specify a return type in your fork?

@darkhelmet
Copy link
Author

All the return types for the methods that build the args show Strings, where now they would be Array. Just a documentation thing.

@tcocca
Copy link
Owner

tcocca commented Dec 20, 2011

OK sounds good, looking forward to your patch/pull request.

@elmatou
Copy link
Contributor

elmatou commented Dec 20, 2011

Hi Guys,

Daniel, As you mentioned, I see that we can build the system call around an array of values, ok. But what is the point ? can you be more precise on the security issue, I didn't find any references about this.

@darkhelmet
Copy link
Author

This is security 101: http://cwe.mitre.org/data/definitions/78.html

@elmatou
Copy link
Contributor

elmatou commented Dec 20, 2011

Thx,
I finaly get the point on the shell expension trick.
We definitivly should pass the command as an array.

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