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.

Sign up for GitHub

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

Jump to bottom

Sinatra or sinatra-contrib has to do path traversal prevention #310

Closed
rkh opened this issue Jun 25, 2011 · 7 comments
Closed

Sinatra or sinatra-contrib has to do path traversal prevention #310

rkh opened this issue Jun 25, 2011 · 7 comments
Labels
vulnerability
Milestone
1.3.0

Comments

@rkh
Copy link
Member

rkh commented Jun 25, 2011

Sinatra 1.3 will follow RFC 2616 for matching patterns to URLs. This introduces a path traversal vulnerability in all applications that use params directly in rendering methods (like erb(params[:template].to_sym)). A counter measure is implemented in rack-protection. I think either Sinatra or sinatra-contrib should depend on it. I would favor sinatra-contrib, but that means we can't release Sinatra 1.3 until we finish sinatra-contrib.
@rkh
Copy link
Member Author

rkh commented Jul 25, 2011

@sr, @bmizerany, @rtomayko opinions? also, directory traversal seems to be the most common attack.

@namelessjon
Copy link

As an aside: If you're calling params[:template].to_sym then you should be aware you'll be leaking memory as symbols are never GCd. One could just try a lot of different templates ...

I'd favour sinatra depending on it, or VERY clear warnings about this in the readme.

@rkh
Copy link
Member Author

rkh commented Jul 25, 2011

There is a third option: Basically copying the path traversal code from rack-protection into Sinatra.

@rkh
Copy link
Member Author

rkh commented Aug 18, 2011

@bmizerany?

@rkh
Copy link
Member Author

rkh commented Sep 2, 2011

I added this to sinatra-contrib for now, but (esp. after a discussion with Sinatra users at Rocky Mountain Ruby) tend to actually include it into Sinatra proper.

@rkh rkh closed this as completed in 1f1e58e Sep 2, 2011
@jacobo
Copy link

jacobo commented Sep 22, 2011

People really do erb(params[:template].to_sym) ?

what about people that like to: redirect params[:return_to] ?

@rkh
Copy link
Member Author

rkh commented Sep 22, 2011

@jacobo: I will disable escaped_params by default, either in Sinatra or rack-protection. Then both scenarios should work.

rkh pushed a commit that referenced this issue Sep 22, 2011
Don't escape parameters by default in included rack-protection. relat…
c59fad7 
…es to issue #310
rkh added a commit that referenced this issue Sep 22, 2011
@rkh
Merge pull request #361 from engineyard/dont_escape_params
9c4ac4c 
Don't escape parameters by default in included rack-protection (issue #310)
@lanej lanej mentioned this issue Nov 4, 2011
Merged
@dentarg dentarg mentioned this issue May 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
vulnerability
Projects
None yet
Milestone
1.3.0
Development

No branches or pull requests
3 participants
@namelessjon @jacobo @rkh