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

Allow CodeRay to be run when $SAFE=1 #159

Closed
wants to merge 1 commit into from
Closed

Conversation

rubys
Copy link

@rubys rubys commented Nov 24, 2013

Test case:

require 'coderay'
$SAFE=1
puts CodeRay.scan("a=1", :ruby).div

@korny
Copy link
Member

korny commented Nov 24, 2013

Mmh, interesting. I guess Ruby's telling us that File operations on dynamic paths are insecure? If so, is it wise to just use untaint here? I'm just not sure if this is the correct fix, or circumventing good security…

@rubys
Copy link
Author

rubys commented Nov 24, 2013

Ruby considers operations involving __FILE__ to be insecure. The "right" fix is to eliminate such usages. This advice is routinely ignored.

The patch I provided is limited to three specific usages of __FILE__. Usages that are combined with other strings and the result passed to File.exist? in load_plugin_map and require in make_plugin_hash.

An alternate approach that I discovered after I posted this patch is to untaint only CodeRay::CODERAY_PATH, and change the three usages of File.dirname(__FILE__) to use this value instead.

@ghost
Copy link

ghost commented Jan 21, 2014

require 'foo/bar' works for me all the time - I always install into my ruby SITE_DIR

Do you think your solution using File.dirname(FILE).untaint is ok to use?

@rubys
Copy link
Author

rubys commented Jan 21, 2014

At the moment, I'm using an even more constrained approach:

https://github.com/rubys/wunderbar/blob/e9e4f295c529461f5518591db08628609515fa56/lib/wunderbar/coderay.rb#L4

@korny
Copy link
Member

korny commented Feb 13, 2016

Ruby now has a __DIR__ token. Maybe we can use this?

@korny korny self-assigned this Feb 13, 2016
@korny korny added the Bug label Feb 13, 2016
@korny korny modified the milestone: 1.1.1 Feb 13, 2016
@korny
Copy link
Member

korny commented Feb 13, 2016

The problem doesn't seem to exist anymore! I'm not sure why.

@korny
Copy link
Member

korny commented Feb 13, 2016

I'll close this one. If anybody can come up with a way to reproduce it, please comment here.

@korny korny closed this Feb 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants