Permalink
Browse files

Properly escape glob characters.

  • Loading branch information...
1 parent 521c9aa commit e0c03f8a2e727619c1eb1822d100d8c920d8bf12 @tenderlove tenderlove committed Aug 16, 2011
@@ -63,7 +63,7 @@ def build_path(name, prefix, partial, details)
end
def query(path, exts, formats)
- query = File.join(@path, path)
+ query = escape_entry File.join(@path, path)
exts.each do |ext|
query << '{' << ext.map {|e| e && ".#{e}" }.join(',') << ',}'
@@ -88,6 +88,10 @@ def query(path, exts, formats)
templates
end
+ def escape_entry(entry)
+ entry.gsub(/(\*|\[|\]|\{|\}|\?)/, "\\\\\\1")
+ end
+
# Extract handler and formats from path. If a format cannot be a found neither
# from the path, or the handler, we should return the array of formats given
# to the resolver.
@@ -396,6 +396,14 @@ def render_with_explicit_template
render :template => "test/hello_world"
end
+ def render_with_explicit_unescaped_template
+ render :template => "test/h*llo_world"
+ end
+
+ def render_with_explicit_escaped_template
+ render :template => "test/hello_w*rld"
+ end
+
def render_with_explicit_string_template
render "test/hello_world"
end
@@ -1057,6 +1065,12 @@ def test_render_with_explicit_template
assert_response :success
end
+ def test_render_with_explicit_unescaped_template
+ assert_raise(ActionView::MissingTemplate) { get :render_with_explicit_unescaped_template }
+ get :render_with_explicit_escaped_template
+ assert_equal "Hello w*rld!", @response.body
+ end
+
def test_render_with_explicit_string_template
get :render_with_explicit_string_template
assert_equal "<html>Hello world!</html>", @response.body
@@ -0,0 +1 @@
+Hello w*rld!

3 comments on commit e0c03f8

@Bertg
Contributor
Bertg commented on e0c03f8 Aug 17, 2011

I have some serious issues with this commit. Bot in my source, and conceptually.

We where using the query method to find which templates are matching a specific patern. This would allow us to use a template on a per customer basis. This behaviour is now broken.

Also, it is not the query's responsibility to fix the possible abuse in the render method. if anything the render method should escape the value passed down.

@pixeltrix
Member

This is related to this vulnerability so I wouldn't expect it to be changed. The best solution is to write your own template resolver and it's covered in @josevalim's book - Crafting Rails Applications or there's a blog post that provides an overview.

@Bertg
Contributor
Bertg commented on e0c03f8 Aug 17, 2011

The PathResolver provides the "query" method. It's the asking code that should pass a "safe" string, or to tell the resolver to do it safe.

We already got it fixed locally with a patch that provides the previous methods under a different name. We won't be "properly" investigating this issue until we move to rails 3.1 as that version has a rewrite of path resolving etc...

Please sign in to comment.