2019 Q1 Recurity Assessment: OAuth CSRF

https://gitlab.com/gitlab-com/gl-security/engineering/issues/329

Details

The state parameter used for the Oauth login flow on the secondary node is not suitable for the intended purpose within Oauth. The main purpose of this parameter is to mitigate CSRF attacks. This, however, is not prevented on the secondary node, as the state parameter is not bound to any user session and might even be replayed.

Reproduction Steps

Observe the validation of the state parameter in ee/lib/gitlab/geo/oauth/login_state.rb:

def valid?
return false unless salt.present? /& hmac.present?
hmac == generate_hmac
end

The validity is not bound to any session. Therefore, previous values might be reused for CSRF attacks.

Recommendation

Recurity Labs recommends to bind the state parameter to the respective user session in order to mitigate CSRF attacks in the Oauth login flow.