Re: [myexperiment-hackers] [2394] trunk/app: Fix for case 98981 - javascript injection in Pack name, reported by Jits.

[Danius Michaelides]

On Wed, 28 Apr 2010, Jiten Bhagat wrote:

> Danius Michaelides wrote:
> > On Wed, 28 Apr 2010, Jiten Bhagat wrote:
> >
> > > Danius Michaelides wrote:
> > > > On Tue, 27 Apr 2010, Jiten Bhagat wrote:
> > > >
> > > > > Hi Dan,
> > > > >
> > > > > Just a note that with the tooltips, the idea is to sometimes allow
> > > > > HTML,
> > > > > so that richer tooltips can be shown (ie: with lists, bold text,
> > > > > italic
> > > > > text, etc). By changing it in the core method this might "break"
> > > > > existing tooltips?
> > > >
> > > > No, I specifically checked for this - I believe I found all the cases.
> > >
> > > OK.
> > >
> > > Though I am using HTML in the tooltips I generate for the service
> > > monitoring status (to match the same way we do it in BioCatalogue). So
> > > this will be affected when the branch is merged back in.
> >
> > I dont follow. HTML in tooltips still work - i've not had to make any
> > changes to them.
>
> I may be missing something here... I see that now any 'text' provided to
> the tooltip_title_attrib(..) method is html escaped, so if I did
> something like:
>
> link_to "My Link With a Tooltip", url, :title =>
> tooltip_title_attrib("<b>My tooltip</b><br/><p>....</p>")
>
> ... then the <b> and <p> tags will be html escaped and therefore not
> rendered in the browser as HTML tags?

tooltip_title_attrib() packs the text into an attribute, that boxover 
uses when it builds the tooltip. The html escaped text is unescaped
when it renders it into the floating tooltip div.

Infact, things like info_icon_with_tooltip already html escape the text
if necessary (I guess image_tag is doing this), but other users of
tooltip_title_attrib() dont seem to.

Danius


> Jits
>
> > Danius
> >
> > > Jits
> > >
> > > > Danius
> > > >
> > > > > Cheers,
> > > > > Jits
> > > > >
> > > > >
> > > > > address@hidden wrote:
> > > > > > Revision
> > > > > >     2394
> > > > > > Author
> > > > > >     dtm
> > > > > > Date
> > > > > >     2010-04-27 12:18:07 -0400 (Tue, 27 Apr 2010)
> > > > > >
> > > > > >
> > > > > >       Log Message
> > > > > >
> > > > > > Fix for case 98981 - javascript injection in Pack name, reported by
> > > > > > Jits.
> > > > > > Fix for javascript injection in tooltips.
> > > > > >
> > > > > >
> > > > > >       Modified Paths
> > > > > >
> > > > > >     * trunk/app/helpers/application_helper.rb
> > > > > >       <#trunkapphelpersapplication_helperrb>
> > > > > >     * trunk/app/views/group_announcements/index.rhtml
> > > > > >       <#trunkappviewsgroup_announcementsindexrhtml>
> > > > > >     * trunk/app/views/networks/_announcements.rhtml
> > > > > >       <#trunkappviewsnetworks_announcementsrhtml>
> > > > > >
> > > > > >
> > > > > >       Diff
> > > > > >
> > > > > >
> > > > > >         Modified: trunk/app/helpers/application_helper.rb (2393 =>
> > > > > > 2394)
> > > > > >
> > > > > >
> > > > > > --- trunk/app/helpers/application_helper.rb    2010-04-27 15:41:01
> > > > > > UTC (rev 2393)
> > > > > > +++ trunk/app/helpers/application_helper.rb    2010-04-27 16:18:07
> > > > > > UTC (rev 2394)
> > > > > > @@ -390,7 +390,7 @@
> > > > > >        end
> > > > > >      when "Pack"
> > > > > >        if p = Pack.find(:first, :conditions => ["id = ?",
> > > > > > contributableid])
> > > > > > -        return link ? link_to(p.title, pack_url(p)) : h(p.title)
> > > > > > +        return link ? link_to(h(p.title), pack_url(p)) : h(p.title)
> > > > > >        else
> > > > > >          return nil
> > > > > >        end
> > > > > > @@ -990,7 +990,7 @@
> > > > > >    end
> > > > > >
> > > > > >    def tooltip_title_attrib(text, delay=200)
> > > > > > -    return "header=[] body=[#{text}]
> > > > > > cssheader=[boxoverTooltipHeader] cssbody=[boxoverTooltipBody]
> > > > > > delay=[#{delay}]"
> > > > > > +    return "header=[] body=[#{h(text)}]
> > > > > > cssheader=[boxoverTooltipHeader] cssbody=[boxoverTooltipBody]
> > > > > > delay=[#{delay}]"
> > > > > >    end
> > > > > >
> > > > > >    # This method checks to see if the current user is allowed to
> > > > > > approve a membership that is still pending approval
> > > > > >
> > > > > >
> > > > > >         Modified: trunk/app/views/group_announcements/index.rhtml
> > > > > >         (2393 => 2394)
> > > > > >
> > > > > >
> > > > > > --- trunk/app/views/group_announcements/index.rhtml    2010-04-27
> > > > > > 15:41:01 UTC (rev 2393)
> > > > > > +++ trunk/app/views/group_announcements/index.rhtml    2010-04-27
> > > > > > 16:18:07 UTC (rev 2394)
> > > > > > @@ -5,7 +5,7 @@
> > > > > >  <% end %>
> > > > > >
> > > > > >  <h1>
> > > > > > -    <%= feed_icon_tag "Group address@hidden Announcements",
> > > > > > formatted_group_announcements_path(@group, :rss) %>
> > > > > > +    <%= feed_icon_tag "Group #{h(@group.title)} Announcements",
> > > > > > formatted_group_announcements_path(@group, :rss) %>
> > > > > >      <%= @group.announcements_in_public_mode_for_user(current_user)
> > > > > > ? "Public " : "All " -%> Group Announcements (<%=
> > > > > > @announcements.length %>)
> > > > > >      <br/>
> > > > > >      <span style="font-size: 77%;">for group: <%= link_to_function
> > > > > > h(@group.title) + expand_image, visual_effect(:toggle_blind,
> > > > > > "group_box", :duration => 0.3) -%></span>
> > > > > >
> > > > > >
> > > > > >         Modified: trunk/app/views/networks/_announcements.rhtml (2393
> > > > > >         => 2394)
> > > > > >
> > > > > >
> > > > > > --- trunk/app/views/networks/_announcements.rhtml    2010-04-27
> > > > > > 15:41:01 UTC (rev 2393)
> > > > > > +++ trunk/app/views/networks/_announcements.rhtml    2010-04-27
> > > > > > 16:18:07 UTC (rev 2394)
> > > > > > @@ -6,7 +6,7 @@
> > > > > >
> > > > > >      <p class="heading" style="margin: 0;">
> > > > > >          <span style="position: relative; z-index: 1000; float:
> > > > > > left;">
> > > > > > -            <%= feed_icon_tag "#{group.title} Group Announcements",
> > > > > > formatted_group_announcements_path(group, :rss) -%>
> > > > > > +            <%= feed_icon_tag "#{h(group.title)} Group
> > > > > > Announcements", formatted_group_announcements_path(group, :rss) -%>
> > > > > >          </span>
> > > > > >          <a name="group_announcements"></a>
> > > > > >          <%= link_to "Announcements", group_announcements_url(group)
> > > > > > -%>
> > > > > > ------------------------------------------------------------------------
> > > > > >
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > myexperiment-hackers mailing list
> > > > > > address@hidden
> > > > > > http://lists.nongnu.org/mailman/listinfo/myexperiment-hackers
> > > > >
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > myexperiment-hackers mailing list
> > > > > address@hidden
> > > > > http://lists.nongnu.org/mailman/listinfo/myexperiment-hackers
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > myexperiment-hackers mailing list
> > > > address@hidden
> > > > http://lists.nongnu.org/mailman/listinfo/myexperiment-hackers
> > >
> > >
> > >
> > > _______________________________________________
> > > myexperiment-hackers mailing list
> > > address@hidden
> > > http://lists.nongnu.org/mailman/listinfo/myexperiment-hackers
> >
> >
> >
> > _______________________________________________
> > myexperiment-hackers mailing list
> > address@hidden
> > http://lists.nongnu.org/mailman/listinfo/myexperiment-hackers
>
>
>
> _______________________________________________
> myexperiment-hackers mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/myexperiment-hackers