Return 403 when attachment "folders" are spidered. Fixes #340

3 files changed, 16 insertions, 4 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 8fd2da54a..05f88a6b2 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -11,6 +11,8 @@
 require 'open-uri'
 
 class ApplicationController < ActionController::Base
+    class PermissionDenied < StandardError
+    end
     # Standard headers, footers and navigation for whole site
     layout "default"
     include FastGettext::Translation # make functions like _, n_, N_ etc available)
@@ -120,6 +122,8 @@ class ApplicationController < ActionController::Base
         case exception
         when ActiveRecord::RecordNotFound, ActionController::UnknownAction, ActionController::RoutingError
             @status = 404
+        when PermissionDenied
+            @status = 403
         else
             @status = 500
             notify_about_exception exception
@@ -189,7 +193,7 @@ class ApplicationController < ActionController::Base
         return File.exists?(key_path)
     end
     def foi_fragment_cache_read(key_path)
-        cached = File.read(key_path)
+        return File.read(key_path)
     end
     def foi_fragment_cache_write(key_path, content)
         FileUtils.mkdir_p(File.dirname(key_path))
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 6e33fe043..fbd7d24d4 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -600,9 +600,13 @@ class RequestController < ApplicationController
     before_filter :authenticate_attachment, :only => [ :get_attachment, :get_attachment_as_html ]
     def authenticate_attachment
         # Test for hidden
-        incoming_message = IncomingMessage.find(params[:incoming_message_id])
-        if !incoming_message.info_request.user_can_view?(authenticated_user)
-            render :template => 'request/hidden', :status => 410 # gone
+        if request.path =~ /\/$/ 
+            raise PermissionDenied.new("Directory listing not allowed")
+        else
+            incoming_message = IncomingMessage.find(params[:incoming_message_id])
+            if !incoming_message.info_request.user_can_view?(authenticated_user)
+                render :template => 'request/hidden', :status => 410 # gone
+            end
         end
     end
 
diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb
index bfb7e5fb5..f8f9162c1 100644
--- a/spec/integration/errors_spec.rb
+++ b/spec/integration/errors_spec.rb
@@ -45,5 +45,9 @@ describe "When rendering errors" do
         get("/request/#{ir.url_title}")
         response.code.should == "500"
     end
+    it "should render a 403 for attempts at directory listing for attachments" do
+        get("/request/5/response/4/attach/html/3/" )
+        response.code.should == "403"
+    end
 end