From 84dcddec6f68d540dcce9b112a1384cdb043f8e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serdar=20=C3=96zer?= Date: Mon, 2 Nov 2026 10:08:36 +0100 Subject: [PATCH 1/2] fix: prevent expired-token replay on v3 upload endpoints The middleware read upload_start_time from Rack params, which merges query string and POST body. A client could supply a backdated value via query string (?upload_start_time=) to shift the token-expiry reference time, allowing an expired token to pass auth on the v3 package, buildpack, and droplet upload endpoints. Additionally, a missing param caused nil.to_i == 0 which is truthy in Ruby, so the relaxed decoder was always constructed with reference time 0 (Unix epoch), accepting any token with exp > 0. --- middleware/security_context_setter.rb | 12 ++- .../security_context_setter_spec.rb | 78 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/middleware/security_context_setter.rb b/middleware/security_context_setter.rb index 06a24deec0c..3321fbafbac 100644 --- a/middleware/security_context_setter.rb +++ b/middleware/security_context_setter.rb @@ -22,7 +22,7 @@ def call(env) security_context_configurer = @security_context_configurer if request_path && request_path.match(UPLOADS_PATH_REGEX) - upload_start_time = Rack::Request.new(env).params['upload_start_time'].to_i + upload_start_time = upload_start_time_from_nginx(env) if upload_start_time relaxed_token_decoder = VCAP::CloudController::UaaTokenDecoder.new(VCAP::CloudController::Config.config.get(:uaa), alternate_reference_time: upload_start_time) security_context_configurer = VCAP::CloudController::Security::SecurityContextConfigurer.new(relaxed_token_decoder) @@ -56,6 +56,16 @@ def call(env) private + def upload_start_time_from_nginx(env) + raw_value = Rack::Request.new(env).POST['upload_start_time'] + return false if raw_value.blank? + + upload_start_time = raw_value.to_i + return false unless upload_start_time.positive? + + [upload_start_time, Time.now.to_i].min + end + def id_from_token VCAP::CloudController::SecurityContext.token['user_id'] || VCAP::CloudController::SecurityContext.token['client_id'] end diff --git a/spec/unit/middleware/security_context_setter_spec.rb b/spec/unit/middleware/security_context_setter_spec.rb index 6031b6e36ee..6c3cd2c98ca 100644 --- a/spec/unit/middleware/security_context_setter_spec.rb +++ b/spec/unit/middleware/security_context_setter_spec.rb @@ -181,6 +181,84 @@ module Middleware expect { middleware.call(env) }.to raise_error(error) end end + + context 'when the request targets an upload endpoint' do + let(:path_info) { '/v3/packages/0b6a2f0e-9c5a-4c1d-8e2f-1a2b3c4d5e6f/upload' } + let(:upload_env) do + { + 'HTTP_AUTHORIZATION' => 'auth-token', + 'PATH_INFO' => path_info, + 'REQUEST_PATH' => path_info, + 'REQUEST_METHOD' => 'POST', + 'QUERY_STRING' => query_string, + 'CONTENT_TYPE' => content_type, + 'rack.input' => StringIO.new(request_body) + } + end + let(:query_string) { '' } + let(:content_type) { 'application/x-www-form-urlencoded' } + let(:request_body) { '' } + + context 'when upload_start_time is absent from the POST body' do + it 'uses the strict token decoder' do + expect(VCAP::CloudController::UaaTokenDecoder).not_to receive(:new) + + middleware.call(upload_env) + end + end + + context 'when upload_start_time is passed as a query parameter (client-supplied, ignored)' do + let(:query_string) { 'upload_start_time=123' } + + it 'ignores it and uses the strict token decoder' do + expect(VCAP::CloudController::UaaTokenDecoder).not_to receive(:new) + + middleware.call(upload_env) + end + end + + context 'when upload_start_time is in the POST body (as injected by nginx upload module)' do + let(:request_body) { 'upload_start_time=123' } + + it 'uses a relaxed token decoder with the given reference time' do + expect(VCAP::CloudController::UaaTokenDecoder).to receive(:new). + with(anything, alternate_reference_time: 123).and_return(token_decoder) + + middleware.call(upload_env) + end + end + + context 'when upload_start_time in the POST body is in the future' do + let(:request_body) { 'upload_start_time=9999999999' } + + it 'clamps the reference time to now' do + expect(VCAP::CloudController::UaaTokenDecoder).to receive(:new). + with(anything, alternate_reference_time: be_within(2).of(Time.now.to_i)).and_return(token_decoder) + + middleware.call(upload_env) + end + end + + context 'when upload_start_time in the POST body is not a positive integer' do + let(:request_body) { 'upload_start_time=abc' } + + it 'uses the strict token decoder' do + expect(VCAP::CloudController::UaaTokenDecoder).not_to receive(:new) + + middleware.call(upload_env) + end + end + + context 'when upload_start_time in the POST body is empty' do + let(:request_body) { 'upload_start_time=' } + + it 'uses the strict token decoder' do + expect(VCAP::CloudController::UaaTokenDecoder).not_to receive(:new) + + middleware.call(upload_env) + end + end + end end end end From 39b5ee718e0e1a5cd57e0678702e95018a1b640d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serdar=20=C3=96zer?= Date: Tue, 3 Nov 2026 16:35:09 +0100 Subject: [PATCH 2/2] fix: fuzzy matcher fixed with exact time --- spec/unit/middleware/security_context_setter_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/unit/middleware/security_context_setter_spec.rb b/spec/unit/middleware/security_context_setter_spec.rb index 6c3cd2c98ca..6bfe35a5d15 100644 --- a/spec/unit/middleware/security_context_setter_spec.rb +++ b/spec/unit/middleware/security_context_setter_spec.rb @@ -232,10 +232,11 @@ module Middleware let(:request_body) { 'upload_start_time=9999999999' } it 'clamps the reference time to now' do - expect(VCAP::CloudController::UaaTokenDecoder).to receive(:new). - with(anything, alternate_reference_time: be_within(2).of(Time.now.to_i)).and_return(token_decoder) - - middleware.call(upload_env) + Timecop.freeze do + expect(VCAP::CloudController::UaaTokenDecoder).to receive(:new). + with(anything, alternate_reference_time: Time.now.to_i).and_return(token_decoder) + middleware.call(upload_env) + end end end