Skip to content
  • Nick Malcolm's avatar
    Add defense-in-depth against mass assignment in authn/z controllers · a77b872b
    Nick Malcolm authored
    In the future, we might make a change to how we handle user request
    parameters in a way that has unexpected and undesired consequence;
    specifically mass assignment vulnerabilities. (There are currently
    none known). These additional unit tests and/or explicit type-casts are
    intended to defend against that future scenario.
    
    For example: attempting to brute force a password by sending many
    passwords in a single request for a single user should never work. Nor
    should sending multiple OTP codes. The reason they _might_
    inadvertently work is because Ruby / Rails often doesn't mind if you
    send a string or an array of strings. For example:
    
    ```ruby
    # POST /vulnerable?email=fake@attacker.com
    > User.find_by(email: params[:email])
    # User Load (3.4ms)  SELECT "users".* FROM "users" WHERE "users"."email" = 'fake@attacker.com' LIMIT 1
    => nil
    
    # We expect email to be a string, but what if it's not?
    # POST /vulnerable?email[]=fake@attacker.com&email[]=admin@example.com
    > User.find_by(email: params[:email])
    # User Load (1.6ms)  SELECT "users".* FROM "users" WHERE "users"."email" IN ('fake@attacker.com', 'admin@example.com')
    => #<User id:1 @root>
    ```
    
    This work resolves https://gitlab.com/gitlab-org/gitlab/-/issues/442831+
    
    The methodology was to look at authentication & authorization-related
    controllers, and down into any Helpers/Services/etc that are called
    or included.
    a77b872b