From e5f10ab9c097c34a7f0abd7e92a2f384353aee68 Mon Sep 17 00:00:00 2001
From: Jared Norman <jared@super.gd>
Date: Wed, 26 Aug 2026 15:43:36 -0700
Subject: [PATCH] Fix guest-token fixation in OrdersController

An attacker-supplied params[:token] was written straight into the
permanent signed guest_token cookie, rebinding the visitor's cart to
the attacker's order and stamping the attacker's token onto future
guest orders. Authorize the tokenized order view against the request
token instead, and stop writing the cookie from params.
---
 app/controllers/spree/orders_controller.rb    |  7 +------
 .../spree/orders_controller_ability_spec.rb   | 19 ++++++++++---------
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index ea4f9c2e32..65f455d0f4 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/app/controllers/spree/orders_controller.rb
@@ -6,14 +6,13 @@ class OrdersController < Spree::StoreController
 
     respond_to :html
 
-    before_action :store_guest_token
     before_action :assign_order, only: :update
     # note: do not lock the #edit action because that's where we redirect when we fail to acquire a lock
     around_action :lock_order, only: :update
 
     def show
       @order = Spree::Order.find_by!(number: params[:id])
-      authorize! :show, @order, cookies.signed[:guest_token]
+      authorize! :show, @order, params[:token] || cookies.signed[:guest_token]
     end
 
     def update
@@ -102,10 +101,6 @@ def accurate_title
 
     private
 
-    def store_guest_token
-      cookies.permanent.signed[:guest_token] = params[:token] if params[:token]
-    end
-
     def order_params
       if params[:order]
         params[:order].permit(*permitted_order_attributes)
diff --git a/spec/controllers/spree/orders_controller_ability_spec.rb b/spec/controllers/spree/orders_controller_ability_spec.rb
index ce912c704e..c44b11756d 100644
--- a/spec/controllers/spree/orders_controller_ability_spec.rb
+++ b/spec/controllers/spree/orders_controller_ability_spec.rb
@@ -19,19 +19,20 @@ module Spree
 
       before do
         allow(controller).to receive_messages current_order: order
+        cookies.signed[:guest_token] = token
       end
 
       context '#populate' do
         it 'should check if user is authorized for :update' do
           expect(controller).to receive(:authorize!).with(:update, order, token)
-          post :populate, params: { variant_id: variant.id, token: token }
+          post :populate, params: { variant_id: variant.id }
         end
       end
 
       context '#edit' do
         it 'should check if user is authorized for :edit' do
           expect(controller).to receive(:authorize!).with(:edit, order, token)
-          get :edit, params: { token: token }
+          get :edit
         end
       end
 
@@ -39,14 +40,14 @@ module Spree
         it 'should check if user is authorized for :update' do
           allow(order).to receive :update
           expect(controller).to receive(:authorize!).with(:update, order, token)
-          post :update, params: { order: { email: "foo@bar.com" }, token: token }
+          post :update, params: { order: { email: "foo@bar.com" } }
         end
       end
 
       context '#empty' do
         it 'should check if user is authorized for :update' do
           expect(controller).to receive(:authorize!).with(:update, order, token)
-          post :empty, params: { token: token }
+          post :empty
         end
       end
 
@@ -65,15 +66,15 @@ module Spree
 
       context '#show' do
         context 'when token parameter present' do
-          it 'always ooverride existing token when passing a new one' do
-            cookies.signed[:guest_token] = "soo wrong"
+          it 'authorizes the request against the supplied token' do
+            expect(controller).to receive(:authorize!).with(:show, kind_of(Spree::Order), order.guest_token)
             get :show, params: { id: 'R123', token: order.guest_token }
-            expect(cookies.signed[:guest_token]).to eq(order.guest_token)
           end
 
-          it 'should store as guest_token in session' do
+          it 'does not adopt the URL token as the guest_token cookie' do
+            cookies.signed[:guest_token] = 'existing-token'
             get :show, params: { id: 'R123', token: order.guest_token }
-            expect(cookies.signed[:guest_token]).to eq(order.guest_token)
+            expect(cookies.signed[:guest_token]).to eq('existing-token')
           end
         end