aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/request_controller.rb3
-rw-r--r--app/controllers/track_controller.rb2
-rw-r--r--app/controllers/user_controller.rb4
-rw-r--r--app/views/general/blog.html.erb4
-rw-r--r--config/application.rb3
-rw-r--r--config/httpd.conf-example7
-rw-r--r--spec/controllers/general_controller_spec.rb12
-rw-r--r--spec/controllers/request_controller_spec.rb12
-rw-r--r--spec/factories/foi_attchments.rb5
-rw-r--r--spec/factories/incoming_messages.rb8
-rw-r--r--spec/fixtures/files/blog_feed.atom2
-rw-r--r--spec/fixtures/files/interesting.html7
12 files changed, 63 insertions, 6 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 413b74cea..39e7616ed 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -776,6 +776,9 @@ class RequestController < ApplicationController
# Prevent spam to magic request address. Note that the binary
# subsitution method used depends on the content type
@incoming_message.apply_masks!(@attachment.body, @attachment.content_type)
+ if response.content_type == 'text/html'
+ @attachment.body = ActionController::Base.helpers.sanitize(@attachment.body)
+ end
render :text => @attachment.body
end
diff --git a/app/controllers/track_controller.rb b/app/controllers/track_controller.rb
index 7018af03c..4b272797f 100644
--- a/app/controllers/track_controller.rb
+++ b/app/controllers/track_controller.rb
@@ -211,7 +211,7 @@ class TrackController < ApplicationController
track_thing.destroy
end
- redirect_to params[:r]
+ redirect_to URI.parse(params[:r]).path
end
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index b7c8252f5..56f42891d 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -256,7 +256,7 @@ class UserController < ApplicationController
def signout
clear_session_credentials
if params[:r]
- redirect_to params[:r]
+ redirect_to URI.parse(params[:r]).path
else
redirect_to :controller => "general", :action => "frontpage"
end
@@ -596,7 +596,7 @@ class UserController < ApplicationController
end
@user.receive_email_alerts = params[:receive_email_alerts]
@user.save!
- redirect_to params[:came_from]
+ redirect_to URI.parse(params[:came_from]).path
end
private
diff --git a/app/views/general/blog.html.erb b/app/views/general/blog.html.erb
index 5dda7ab74..fd87bd9fe 100644
--- a/app/views/general/blog.html.erb
+++ b/app/views/general/blog.html.erb
@@ -10,9 +10,9 @@
<p class="subtitle"><%= _("Posted on {{date}} by {{author}}", :date=>simple_date(Time.parse(item['pubDate'][0])), :author=> item['creator'] ? item['creator'][0] : item['author'][0]) %></p>
<div>
<% if item['encoded'] %>
- <%= raw item['encoded'][0] %>
+ <%= sanitize(raw item['encoded'][0]) %>
<% elsif item['description'] %>
- <%= raw item['description'][0] %>
+ <%= sanitize(raw item['description'][0]) %>
<% end %>
</div>
<p><em>
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/config/httpd.conf-example b/config/httpd.conf-example
index 1a3729f5d..00722fbdf 100644
--- a/config/httpd.conf-example
+++ b/config/httpd.conf-example
@@ -108,6 +108,13 @@
RewriteCond %{DOCUMENT_ROOT}/views_cache/cy/request/$2/$1/${escape:$3} -f
RewriteRule ^/cy/request/((\d{1,3})\d*)/(response/\d+/attach/(html/)?\d+/.+) /views_cache/cy/request/$2/$1/${escape:$3} [L]
+ # Don't allow anything to execute from the cache
+ <Directory "/var/www/alaveteli/public/views_cache">
+ Options -ExecCGI
+ SetHandler default-handler
+ AllowOverride None
+ </Directory>
+
# Compress assets
<Location />
<IfModule mod_deflate.c>
diff --git a/spec/controllers/general_controller_spec.rb b/spec/controllers/general_controller_spec.rb
index 4a7a0bb48..28dac7b96 100644
--- a/spec/controllers/general_controller_spec.rb
+++ b/spec/controllers/general_controller_spec.rb
@@ -53,6 +53,18 @@ describe GeneralController, 'when getting the blog feed' do
end
end
+ context 'when the blog has entries' do
+
+ render_views
+
+ it 'should escape any javascript from the entries' do
+ controller.stub!(:quietly_try_to_open).and_return(load_file_fixture("blog_feed.atom"))
+ get :blog
+ response.body.should_not include('<script>alert("exciting!")</script>')
+ end
+
+ end
+
end
describe GeneralController, "when showing the frontpage" do
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 15e252501..2d3ccfa63 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 16930b887..b29fe8ce9 100644
--- a/spec/factories/incoming_messages.rb
+++ b/spec/factories/incoming_messages.rb
@@ -26,6 +26,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/blog_feed.atom b/spec/fixtures/files/blog_feed.atom
index f49693938..a831243b4 100644
--- a/spec/fixtures/files/blog_feed.atom
+++ b/spec/fixtures/files/blog_feed.atom
@@ -29,7 +29,7 @@
<guid isPermaLink="false">http://www.example.com/?id=333</guid>
<description><![CDATA[An example post [...]]]></description>
<content:encoded><![CDATA[<h3>A blog post</h3>
-<p>Example post</p>
+<p>Example post</p><script>alert("exciting!")</script>
]]></content:encoded>
<wfw:commentRss>http://www.example.com/feed/</wfw:commentRss>
<slash:comments>2</slash:comments>
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>
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-22 16:43:13 +0000