From 8cab970cd3df9dca50e9a75e860344d62f68b3ef Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terceiro@colivre.coop.br>
Date: Thu, 2 Sep 2010 10:58:14 -0300
Subject: [PATCH] Fix XSS protection in article titles

---
 app/models/article.rb              |  2 +-
 app/models/tiny_mce_article.rb     |  4 ++--
 test/unit/article_test.rb          | 20 ++++++++++++++++++--
 test/unit/tiny_mce_article_test.rb |  5 +++++
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/app/models/article.rb b/app/models/article.rb
index adc97b4..dd05ac8 100644
--- a/app/models/article.rb
+++ b/app/models/article.rb
@@ -26,7 +26,7 @@ class Article < ActiveRecord::Base
     article.published_at = article.created_at if article.published_at.nil?
   end
 
-  xss_terminate :only => [ :name ], :on => 'validation'
+  xss_terminate :only => [ :name ], :on => 'validation', :with => 'white_list'
 
   named_scope :in_category, lambda { |category|
     {:include => 'categories', :conditions => { 'categories.id' => category.id }}
diff --git a/app/models/tiny_mce_article.rb b/app/models/tiny_mce_article.rb
index 491852a..bd13290 100644
--- a/app/models/tiny_mce_article.rb
+++ b/app/models/tiny_mce_article.rb
@@ -8,9 +8,9 @@ class TinyMceArticle < TextArticle
     _('Not accessible for visually impaired users.')
   end
   
-  xss_terminate :except => [ :abstract, :body ]
+  xss_terminate :only => [  ]
 
-  xss_terminate :only => [ :abstract, :body ], :with => 'white_list', :on => 'validation'
+  xss_terminate :only => [ :name, :abstract, :body ], :with => 'white_list', :on => 'validation'
 
   include WhiteListFilter
   filter_iframes :abstract, :body, :whitelist => lambda { profile && profile.environment && profile.environment.trusted_sites_for_iframe }
diff --git a/test/unit/article_test.rb b/test/unit/article_test.rb
index 9f18cc6..45d3783 100644
--- a/test/unit/article_test.rb
+++ b/test/unit/article_test.rb
@@ -862,7 +862,24 @@ class ArticleTest < Test::Unit::TestCase
     article.name = "<h1 Bla </h1>"
     article.valid?
 
-    assert article.errors.invalid?(:name)
+    assert_no_match /<[^>]*</, article.name
+  end
+
+  should 'not doubly escape quotes in the name' do
+    profile = fast_create(Profile)
+    a = fast_create(Article, :profile_id => profile.id)
+    p = PublishedArticle.create!(:reference_article => a, :profile => fast_create(Community))
+
+    p.name = 'title with "quotes"'
+    p.save
+    assert_equal 'title with "quotes"', p.name
+  end
+
+  should 'remove script tags from name' do
+    a = Article.new(:name => 'hello <script>alert(1)</script>')
+    a.valid?
+
+    assert_no_match(/<script>/, a.name)
   end
 
   should 'escape malformed html tags' do
@@ -878,5 +895,4 @@ class ArticleTest < Test::Unit::TestCase
     article.name = 'a123456789abcdefghij'
     assert_equal 'a123456789ab...', article.short_title
   end
-
 end
diff --git a/test/unit/tiny_mce_article_test.rb b/test/unit/tiny_mce_article_test.rb
index cf14259..6f9909d 100644
--- a/test/unit/tiny_mce_article_test.rb
+++ b/test/unit/tiny_mce_article_test.rb
@@ -113,4 +113,9 @@ class TinyMceArticleTest < Test::Unit::TestCase
     assert_match  /<!-- .* --> <h1> Wellformed html code <\/h1>/, article.body
   end
 
+  should 'not allow XSS on name' do
+    article = TinyMceArticle.create!(:name => 'title with <script>alert("xss")</script>', :profile => profile)
+    assert_no_match /script/, article.name
+  end
+
 end
--
libgit2 0.21.2