Merge pull request #1077 from opf/fix/back_url_verification_again

app/controllers/application_controller.rb

@@ -423,7 +423,8 @@ class ApplicationController < ActionController::Base
 
   def redirect_back_or_default(default)
     back_url = URI.escape(CGI.unescape(params[:back_url].to_s))
-    if !back_url.blank?
+    # if we have a back_url it must not contain two consecutive dots
+    if back_url.present? && !back_url.match(%r{\.\.})
       begin
         uri = URI.parse(back_url)
 
@@ -434,7 +435,10 @@ class ApplicationController < ActionController::Base
         # do not redirect user to the login or register page
         uri_path_allowed  = !uri.path.match(%r{/(login|account/register)})
 
-        if uri_local_to_host && uri_path_allowed
+        # do not redirect to another subdirectory
+        uri_subdir_allowed = relative_url_root.blank? || uri.path.match(%r{\A#{relative_url_root}})
+
+        if uri_local_to_host && uri_path_allowed && uri_subdir_allowed
           redirect_to(back_url)
           return
         end

spec/controllers/account_controller_spec.rb

@@ -72,6 +72,46 @@ describe AccountController do
         user.current_password.should be_nil
       end
 
+      context 'with a relative url root' do
+        before do
+          @old_relative_url_root, ApplicationController.relative_url_root = ApplicationController.relative_url_root, "/openproject"
+        end
+
+        after do
+          ApplicationController.relative_url_root = @old_relative_url_root
+        end
+
+        it "should redirect to the same subdirectory with an absolute path" do
+          post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http://test.host/openproject/work_packages/show/1'}
+          expect(response).to redirect_to '/openproject/work_packages/show/1'
+        end
+
+        it "should redirect to the same subdirectory with a relative path" do
+          post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => '/openproject/work_packages/show/1'}
+          expect(response).to redirect_to '/openproject/work_packages/show/1'
+        end
+
+        it "should not redirect to another subdirectory with an absolute path" do
+          post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http://test.host/foo/work_packages/show/1'}
+          expect(response).to redirect_to '/my/page'
+        end
+
+        it "should not redirect to another subdirectory with a relative path" do
+          post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => '/foo/work_packages/show/1'}
+          expect(response).to redirect_to '/my/page'
+        end
+
+        it "should not redirect to another subdirectory by going up the path hierarchy" do
+          post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http://test.host/openproject/../foo/work_packages/show/1'}
+          expect(response).to redirect_to '/my/page'
+        end
+
+        it "should not redirect to another subdirectory with a protocol relative path" do
+          post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => '//test.host/foo/work_packages/show/1'}
+          expect(response).to redirect_to '/my/page'
+        end
+      end
+
     end
   end