Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #6794 (closed defect: fixed)

Opened 2 years ago

Last modified 8 months ago

TextHelper#sanitize generates invalid HTML; XSS vulnerable

Reported by: tietew Assigned to: technoweenie
Priority: normal Milestone: 2.x
Component: ActionPack Version: 1.2.0rc1
Severity: critical Keywords: xss
Cc:

Description

This vulnerability is found in all versions of ActionView.

In any view, 

<%= sanitize(%q[<SPAN title="'><script>alert()</script>">blah</SPAN>]) %>

returns: 

<span title='\'><script>alert()</script>'>blah</span>

This is invalid HTML, and SCRIPT WORKS!! This must be, at least, 

<span title="'><script>alert()</script>">blah</span>

or 

<span title='&#39;><script>alert()</script>'>blah</span>

Best result is: 

<span title="'&gt;&lt;script&gt;alert()&lt;/script&gt;">blah</span>

Change History

(in reply to: ↑ description ) 12/08/06 06:39:07 changed by rick

Hi tietw, I wrote the white_list plugin to solve this issue. Though, I'm not sure how the html tokenizer would handle that broken html. hmm. If the plugin works well enough, I'd like to replace sanitize with it, as I think a white list approach is better than a black list.

(follow-up: ↓ 3 ) 12/08/06 07:05:27 changed by tietew

I tried white_list plugin but I got same result. 

$ script/console
Loading development environment.
>> include WhiteListHelper
=> Object
>> white_list(%q[<SPAN title="'><script>alert()</script>">blah</SPAN>])
=> "<span title='\\'><script>alert()</script>'>blah</span>"

Including "<" and ">" in value of a tag attribute is allowed. <SPAN title="<>"> is a valid HTML tag.

Problem is that TextHelper#sanitize replaces %q(') into %q(\'). This behavior is incorrect escaping for HTML.

(in reply to: ↑ 2 ) 12/20/06 20:49:01 changed by rick

  • owner changed from core to technoweenie.
  • status changed from new to assigned.

Crazy, I'll add those failing test cases. This looks like a bug in the html tokenizer itself though.

01/26/07 05:24:36 changed by technoweenie

I fixed this in white_list. I just make sure to html escape attributes too. +1 for adding the plugin to core...

11/26/07 03:40:39 changed by technoweenie

  • status changed from assigned to closed.
  • resolution set to fixed.

this was added in core for rails 2.0 awhile ago...