Yes, Brakeman should probably not warn on to_param/to_query.
On 04/11/2013 01:36 PM, Matthew Brookes wrote:
> Thanks Justin,
>
> A quick search for CGI.escape brought me to this discussion
> <http://stackoverflow.com/a/13059657/1447810> on Ruby escape methods,
> which in one of the comments had a link to another discussion, which
> mentions <http://stackoverflow.com/a/13626484/1447810> the ActiveSupport
> to_query</> CGI.escape wrapper, which among it's various forms lets me use:
>
> <%= link_to 'View in Google Earth',
> earth_index_path<<'?'<<{location_id: @location.id
> <http://location.id>}.to_query %>
>
> ...which escapes and builds the query string(s), so should be safe.
>
> Brakeman still has other ideas though:
>
> Unsafe model attribute in link_to href near line 12: link_to("View
> in Google Earth", ((earth_index_path << "?") << { :location_id =>
> (+Location.find(params[:id])+.id) }.to_query))
>
> ...but Brankeman's code expansion made me realise I should probably be
> using params[:id] directly (I'm using @location for other things in the
> view, but still...):
>
> <%= link_to 'View in Google Earth',
> earth_index_path<<'?'<<{location_id: params[:id]}.to_query %>
>
> Brakeman still has other ideas though:
>
> Unsafe parameter value in link_to href near line 12: link_to("View
> in Google Earth", ((earth_index_path << "?") << { :location_id =>
> (+params+[:id]) }.to_query))
>
> It doesn't like these forms either:
>
> params[:id].to_query(:location_id)
> params[:id].to_query('location_id')
> {:location_id => params[:id]}.to_query
> {location_id: params[:id]}.to_param
>
> So I tried plain CGI.escape as you suggested, and hey presto! So it
> seems brakeman doesn't know about to_query / to_param.
>
> Matt.
>
>
>
>
> On 11 April 2013 19:01, Justin Collins <jus...@presidentbeef.com
> <mailto:jus...@presidentbeef.com>> wrote:
>
> Neil should probably confirm, but I believe the way to approach this
> would be to use CGI.escape:
>
> <%= link_to "View in Google Earth", earth_index_path << "?location_id="
> << CGI.escape(@location.to_id.to_s) %>
>
>
> On 04/11/2013 10:13 AM, Matthew Brookes wrote:
> > Ach, sorry, my mistake - when I expanded the terminal window to
> copy as
> > much of the warning as possible, I realised it was actually a link_to
> > warning for the following line, caused by this rather ugly piece
> of code:
> >
> > <%= link_to "View in Google Earth",
> > earth_index_path<<"?location_id="<<@location.id.to_s %>
> >
> > The earth#index view linked to here embeds the google-earth
> plugin, and
> > passes the location id referenced by params[:location_id] as another
> > query string back to a location resource as a networkLink
> request. This
> > in turn sends the lat/lon (among other things) as kml to tell the
> google
> > earth plugin where to center the view, and what to overlay. It's
> a bit
> > of a daisy-chain, but it works!
> >
> > I could possibly use session / flash to pass the location_id, but
> using
> > a querystring makes the link bookmarkable. Also, i /think /the
> > google-earth plugin maintains its own session. Any ideas?
> >
> > Thanks!
> >
> >
> > On 11 April 2013 01:01, Justin Collins <jus...@presidentbeef.com
> <mailto:jus...@presidentbeef.com>
> > <mailto:jus...@presidentbeef.com
> <mailto:jus...@presidentbeef.com>>> wrote:
> >
> > Actually, image_tag (and most other _tag methods) should be
> ignored.
> >
> > I'm having trouble reproducing this warning. Can you show us
> the entire
> > warning output? What version of Rails and Brakeman are you using?
> >
> > Thanks!
> >
> > -Justin
> >
> > On 04/10/2013 04:06 PM, Matthew Brookes wrote:
> > > Hi!
> > >
> > > I'm getting an XSS warning for this:
> > >
> > > <%= image_tag
> > >
> >
>
> "http://maps.google.com/maps/api/staticmap?size=610x450&sensor=false&zoom=15&markers=#{@location.latitude}%2C#{@location.longitude}";
> > > %>
> > >
> > > Is there something I need to do to improve my code, or is
> this an
> > > expected false positive?
> > >
> > > Thanks!
> > > Matt.
> > >
> > >
> > >
> > >
> > > On 10 April 2013 18:09, Matthew Brookes <m...@brookes.net
> <mailto:m...@brookes.net>
> > <mailto:m...@brookes.net <mailto:m...@brookes.net>>
> > > <mailto:m...@brookes.net <mailto:m...@brookes.net>
> <mailto:m...@brookes.net <mailto:m...@brookes.net>>>> wrote:
> > >
> > >
> > >
> >
> >
>
>
