Re: [brakeman] Re:

[Matthew Brookes]

Got it. I submitted an issue, but when I RTFM'd (I should do that more
often!) link_to, it turns it can generate (I presume safely escaped?) query
strings, so I ended up with:

<%= link_to 'View in Google Earth', earth_index_path(:location_id =>
params[:id]) %> which looks cleaner and keeps brakeman happy. :-)

Thanks!

Matt.


On 12 April 2013 15:53, Justin Collins  wrote:

> 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
> >  on Ruby escape methods,
> > which in one of the comments had a link to another discussion, which
> > mentions  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
> > }.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  > > 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  > 
> >  >  > >> 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?
> >  >  >
> >  

Re: [brakeman] Re:

[Justin Collins]

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
>  on Ruby escape methods,
> which in one of the comments had a link to another discussion, which
> mentions  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
> }.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  > 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  
>  >  >> 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  
>  > >
>  >  > 
>   >  >
>  >  >
>  >  >
>  >
>  >
>
>

Re: [brakeman] Re:

[Matthew Brookes]

Thanks Justin,

A quick search for CGI.escape brought me to this
discussionon Ruby escape
methods, which in one of the comments had a link to another
discussion, which mentions
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}.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  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  > > 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  > 
> >  > >> wrote:
> >  >
> >  >
> >  >
> >
> >
>
>

Re: [brakeman] Re:

[Justin Collins]

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  > 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  
>  > >> wrote:
>  >
>  >
>  >
>
>

Re: [brakeman] Re:

[Matthew Brookes]

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  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  > > wrote:
> >
> >
> >
>
>

Re: [brakeman] Re:

[Justin Collins]

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  > wrote:
>
>
>

Re: [brakeman] Re:

[Neil Matatall]

Rails 2?

If so, those interpolated values would need to be h()'d

On Wed, Apr 10, 2013 at 4: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  wrote:
>>
>>
>

[brakeman] Re:

[Matthew Brookes]

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  wrote:

>
>