Sanitize the contents of HTML attachments before display

6 files changed, 39 insertions, 0 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 9e2c291dc..a334abcb7 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -777,6 +777,10 @@ class RequestController < ApplicationController
         # we don't use @attachment.content_type here, as we want same mime type when cached in cache_attachments above
         response.content_type = AlaveteliFileTypes.filename_to_mimetype(params[:file_name]) || 'application/octet-stream'
 
+        if response.content_type == 'text/html'
+            @attachment.body = ActionController::Base.helpers.sanitize(@attachment.body)
+        end
+
         render :text => @attachment.body
     end
 
diff --git a/config/application.rb b/config/application.rb
index ed4f07819..366077795 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -36,6 +36,9 @@ module Alaveteli
     # JavaScript files you want as :defaults (application.js is always included).
     # config.action_view.javascript_expansions[:defaults] = %w(jquery rails)
 
+    # Allow some extra tags to be whitelisted in the 'sanitize' helper method
+    config.action_view.sanitized_allowed_tags = 'html', 'head', 'body', 'table', 'tr', 'td', 'style'
+
     # Configure the default encoding used in templates for Ruby 1.9.
     config.encoding = "utf-8"
 
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 6c0f4573e..26e46a966 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -596,6 +596,18 @@ describe RequestController, "when showing one request" do
             response.status.should == 303
         end
 
+        it "should sanitise HTML attachments" do
+            incoming_message = FactoryGirl.create(:incoming_message_with_html_attachment)
+            get :get_attachment, :incoming_message_id => incoming_message.id,
+                                 :id => incoming_message.info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.html',
+                                 :skip_cache => 1
+            response.body.should_not match("script")
+            response.body.should_not match("interesting")
+            response.body.should match('dull')
+        end
+
         it "should censor attachments downloaded as binary" do
             ir = info_requests(:fancy_dog_request)
 
diff --git a/spec/factories/foi_attchments.rb b/spec/factories/foi_attchments.rb
index 4e9875a00..a1d04ccf0 100644
--- a/spec/factories/foi_attchments.rb
+++ b/spec/factories/foi_attchments.rb
@@ -16,6 +16,11 @@ FactoryGirl.define do
             filename 'interesting.rtf'
             body { load_file_fixture('interesting.rtf') }
         end
+        factory :html_attachment do
+            content_type 'text/html'
+            filename 'interesting.html'
+            body { load_file_fixture('interesting.html') }
+        end
     end
 
 end
diff --git a/spec/factories/incoming_messages.rb b/spec/factories/incoming_messages.rb
index 38ad98394..ec0afdcd0 100644
--- a/spec/factories/incoming_messages.rb
+++ b/spec/factories/incoming_messages.rb
@@ -23,6 +23,14 @@ FactoryGirl.define do
             end
         end
 
+        factory :incoming_message_with_html_attachment do
+            after_create do |incoming_message, evaluator|
+                FactoryGirl.create(:html_attachment,
+                                   :incoming_message => incoming_message,
+                                   :url_part_number => 2)
+            end
+        end
+
         factory :incoming_message_with_attachments do
             # foi_attachments_count is declared as an ignored attribute and available in
             # attributes on the factory, as well as the callback via the evaluator
diff --git a/spec/fixtures/files/interesting.html b/spec/fixtures/files/interesting.html
new file mode 100644
index 000000000..4227eab45
--- /dev/null
+++ b/spec/fixtures/files/interesting.html
@@ -0,0 +1,7 @@
+<html>
+  <head>
+  </head>
+  <body>dull
+    <script>alert('interesting')</script>
+  </body>
+</html>