Commit 33554c93 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Don't modify env in request forgery protection

This avoids modifying the Rack-env in request forgery protection.

If we do allow the env to be modified, this would cause requests made
to our public API by our own frontend to be incorrectly recorded in
metrics and logs.

The Gitlab::RequestForgeryProtection::Controller and it's index action
would be recorded as the caller instead of the actual endpoint being
called.
parent e8c3cd84
...@@ -23,7 +23,9 @@ module Gitlab ...@@ -23,7 +23,9 @@ module Gitlab
end end
def self.verified?(env) def self.verified?(env)
call(env) minimal_env = env.slice('REQUEST_METHOD', 'rack.session', 'HTTP_X_CSRF_TOKEN')
.merge('rack.input' => '')
call(minimal_env)
true true
rescue ActionController::InvalidAuthenticityToken rescue ActionController::InvalidAuthenticityToken
......
...@@ -52,6 +52,11 @@ RSpec.describe Gitlab::RequestForgeryProtection, :allow_forgery_protection do ...@@ -52,6 +52,11 @@ RSpec.describe Gitlab::RequestForgeryProtection, :allow_forgery_protection do
end end
describe '.verified?' do describe '.verified?' do
it 'does not modify the env' do
env['REQUEST_METHOD'] = "GET"
expect { described_class.verified?(env) }.not_to change { env }
end
context 'when the request method is GET' do context 'when the request method is GET' do
before do before do
env['REQUEST_METHOD'] = 'GET' env['REQUEST_METHOD'] = 'GET'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment