jgbailey
1/25/2006 2:02:00 AM
You got me thinking with that. The ERB solution was too broad, so I
looked at the view itself. What about the instance variables? Many
times, those are how you are getting information back to the page
(either through params or direct copy of instance variables from the
controller to the view).
My idea was to iterate through the instance variables on the view and,
for any tainted ones, override their to_s method so it called
CGI::escapeHTML. With a little hacking (and the realization I needed to
look inside Hashes and Arrays), I came up with the below. Put an HTML
element in the 'name' field and it will tell you what you submitted,
properly escaped. Go on - try a <script> tag - I dare you!
<%
def escape_tainted(val)
if val.is_a?(Array)
return val.each { |v| escape_tainted(v) }.any?
elsif val.is_a?(Hash)
return val.each { |v, k | escape_tainted(v) || escape_tainted(k)
}.any?
elsif val.tainted?
class << val
def to_s
CGI::escapeHTML(super)
end
end
return true
else
return false
end
end
self.instance_variables.each { |o|
escape_tainted(instance_variable_get(o)) }
%>
<%= start_form_tag :action => "form_test" %>
Name: <input type="text" name="name" value="<%=@params["name"]%>"><br>
<%= submit_tag "Submit" %><br>
<%= end_form_tag %>
It's still only about 50% of the solution but I did think it was
interesting. One problem with it is that Rails is smart enough to HTML
encode values passed to many of its functions, so you will see
double-escaping. I also noticed that if I dumped something like
<%=params.%> on the page (to see all key-value pairs) the "name" value
inside would NOT be escaped. I don't how that value is displayed
without to_s being called, though.
mental@rydia.net wrote:
> Quoting m4dc4p <jgbailey@gmail.com>:
>
> > I thought it was logical that if a piece of code was tainted, it
> > should be HTML escaped. Unfortuntately, this was a little too
> > broad and it ended up escaping some things I didn't want escaped.
>
> Hmm, that approach makes sense, actually. Maybe you really should
> be inspecting and untainting those additional things.
>
> -mental