Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Comments

Projects
None yet
3 participants

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

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.

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).

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/active_pdftk@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?

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

Owner

tcocca commented Dec 20, 2011

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

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.

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