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

[Jiten Bhagat]

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?
>   

I've just tested this on the services branch and it does still render
the HTML (even though the source has the HTML encoded text). Does this
mean that any HTML escaped content in the tooltips will still be
rendered by the browser, thus allowing for any script injection
regardless of it being html encoded? Or do we need to double html encode
stuff? Or maybe the right thing to do here is use the white_list method
to explicitly get rid of any <script> tags etc?

Cheers,
Jits

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