Avo has a XSS vulnerability on `return_to` param

@@ -4,5 +4,5 @@
 <%= hidden_field_tag :via_record_id, params[:via_record_id] if params[:via_record_id] %>
 <%= hidden_field_tag :via_relation, params[:via_relation] if params[:via_relation] %>
 <%= hidden_field_tag :via_belongs_to_resource_class, params[:via_belongs_to_resource_class] if params[:via_belongs_to_resource_class] %>
-<%= hidden_field_tag :return_to, params[:return_to] if params[:return_to].present? %>
+<%= hidden_field_tag :return_to, e(params[:return_to]) if params[:return_to].present? %>
 <%= hidden_field_tag :referrer, @back_path if params[:via_resource_class] %>

@@ -57,7 +57,7 @@ def edit_path
     # When coming from a turbo frame, we don't want to return to that exact frame
     # for example when editing a has_one field we want to return to the parent frame
     # not the frame of the has_one field.
-    args[:return_to] = request.url unless request.url.include?("turbo_frame=")
+    args[:return_to] = e(request.fullpath) unless request.url.include?("turbo_frame=")
 
     helpers.edit_resource_path(record: @resource.record, resource: @resource, **args)
   end

@@ -326,7 +326,6 @@ def raise_404
 
     def decode_params
       if params[:return_to].present?
-        params[:raw_return_to] = params[:return_to]
         params[:return_to] = d(params[:return_to])
       end
     end

@@ -546,7 +546,9 @@ def redirect_path_from_resource_option(action = :after_update_path)
       return nil if @resource.class.send(action).blank?
 
       extra_args = {}
-      extra_args[:return_to] = params[:return_to] if params[:return_to].present?
+      if params[:return_to].present?
+        extra_args[:return_to] = e(params[:return_to])
+      end
 
       if @resource.class.send(action) == :index
         resources_path(resource: @resource, **extra_args)

@@ -161,14 +161,16 @@ def container_classes
 
     # encode & encrypt params
     def e(value)
-      Avo::Services::EncryptionService.encrypt(message: value, purpose: :return_to, serializer: Marshal)
+      encrypted = Avo::Services::EncryptionService.encrypt(message: value, purpose: :return_to, serializer: Marshal)
+      Base64.urlsafe_encode64(encrypted, padding: false)
     end
 
     # decrypt & decode params
     def d(value)
-      Avo::Services::EncryptionService.decrypt(message: value, purpose: :return_to, serializer: Marshal)
+      decoded = Base64.urlsafe_decode64(value.to_s)
+      Avo::Services::EncryptionService.decrypt(message: decoded, purpose: :return_to, serializer: Marshal)
     rescue
-      value
+      nil
     end
 
     private

@@ -1,4 +1,5 @@
 require "rails_helper"
+require "cgi"
 
 RSpec.describe "App", type: :system do
   let!(:user) { create :user }
@@ -83,5 +84,41 @@
       visit '/admin/resources/projects?per_page=1&turbo_frame=has_many_field_show_test_xgc2pf><script>alert("XSS")<%2Fscript>p9sk5'
       expect { accept_alert }.to raise_error(Capybara::ModalNotFound)
     end
+
+    it "sanitizes unsafe return_to values" do
+      project = projects.first
+      payload = CGI.escape("javascript:alert('Reflected XSS')")
+
+      visit "/admin/resources/projects/#{project.id}?return_to=#{payload}"
+
+      go_back_link = find("div[data-target='panel-tools'] a", text: "Go back")
+      expect(go_back_link[:href]).to end_with("/admin/resources/projects")
+      expect(go_back_link[:href]).not_to start_with("javascript:")
+
+      expect {
+        accept_alert do
+          click_on "Go back"
+        end
+      }.to raise_error(Capybara::ModalNotFound)
+
+      expect(page).to have_current_path "/admin/resources/projects"
+    end
+
+    it "keeps valid internal return_to paths" do
+      project = projects.first
+      encryption_helper = Class.new do
+        include Avo::ApplicationHelper
+      end.new
+      return_to = "/admin/resources/projects/new"
+
+      visit "/admin/resources/projects/#{project.id}?return_to=#{encryption_helper.e(return_to)}"
+
+      go_back_link = find("div[data-target='panel-tools'] a", text: "Go back")
+      expect(go_back_link[:href]).to end_with(return_to)
+
+      click_on "Go back"
+
+      expect(page).to have_current_path return_to
+    end
   end
 end

@@ -1,7 +1,9 @@
 require "rails_helper"
+require "timeout"
 
 RSpec.describe "OpenFieldAttachment", type: :system do
   include ActionView::RecordIdentifier
+
   let!(:user) { create :user }
 
   context "with PDF attachment" do
@@ -30,12 +32,33 @@
         expect(link[:target]).to eq("_blank")
         expect(link[:rel]).to eq("noopener noreferrer")
 
-        new_window = window_opened_by { link.click }
-        expect(new_window).not_to be_nil
+        new_windows = []
+
+        begin
+          existing_windows = page.windows
+          link.click
+
+          Timeout.timeout(Capybara.default_max_wait_time) do
+            loop do
+              new_windows = page.windows - existing_windows
+              break if new_windows.any?
+
+              sleep 0.05
+            end
+          end
 
-        within_window new_window do
-          # Check that final URL ends with filename, since path changes after redirect
-          expect(page.current_url).to match(/dummy-file\.pdf\z/)
+          expect(new_windows).not_to be_empty
+          expect(new_windows.any? do |window|
+            within_window(window) do
+              page.has_current_path?(/dummy-file\.pdf\z/, url: true, wait: 2) || page.current_url.include?(file_path)
+            end
+          end).to be(true)
+        ensure
+          new_windows.each do |window|
+            window.close if page.windows.include?(window)
+          rescue Ferrum::BrowserError, Capybara::WindowError
+            # CI can auto-dispose PDF popup targets before explicit close.
+          end
         end
       end
     end