CVE-2024-21654 (YAML, published 2024-01-12T21:15Z)
Description: value: Rubygems.org is the Ruby community's gem hosting service. Rubygems.org users with MFA enabled would normally be protected from account takeover in the case of email account takeover. However, a workaround on the forgotten password form allows an attacker to bypass the MFA requirement and takeover the account. This vulnerability has been patched in commit 0b3272a.
Severity: nan (CVSS v3 base score: 9.8)
Weakness: CWE-287 \u2014 Improper Authentication
When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
Repository: https://github.com/rubygems/rubygems.org
Files changed:
app/controllers/adoptions_controller.rb
app/controllers/application_controller.rb
app/controllers/concerns/session_verifiable.rb
app/controllers/passwords_controller.rb
app/controllers/sessions_controller.rb
app/views/passwords/edit.html.erb
config/locales/de.yml
config/locales/en.yml
config/locales/es.yml
config/locales/fr.yml
config/locales/ja.yml
config/locales/nl.yml
config/locales/pt-BR.yml
config/locales/zh-CN.yml
config/locales/zh-TW.yml
test/functional/passwords_controller_test.rb
test/integration/password_reset_test.rb
Commit message: Protect forgotten password changes from MFA bypass.

It was previously possible to submit the password update form directly
with only the confirmation token, bypassing MFA protections.


Vulnerable code:
  before_action :redirect_to_verify, if: -> { current_user_is_owner? && !password_session_active? }
  def password_session_active?
    session[:verification] && session[:verification] > Time.current && session.fetch(:verified_user, "") == current_user.id
  end

      before_action :redirect_to_verify, **opts, unless: :password_session_active?
    @user = find_user_for_update

    if @user.update_password password_from_password_reset_params
      @user.reset_api_key! if reset_params[:reset_api_key] == "true"
      @user.api_keys.expire_all! if reset_params[:reset_api_keys] == "true"
      sign_in @user
    session[:verified_user] = current_user.id
    session[:verification] = Gemcutter::PASSWORD_VERIFICATION_EXPIRY.from_now
             :url => user_password_path(@user, :token => @user.confirmation_token),
  <%= error_messages_for @user %>
    context "with reset_api_key and invalid password" do
          password_reset: { reset_api_key: "true", password: "pass" }
      should respond_with :success
    context "without reset_api_key and valid password" do
        put :update, params: {
          user_id: @user.id,
          token: @user.confirmation_token,
          password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD }
        }
      should respond_with :found

      should "not change api_key" do
        assert_equal(@user.reload.api_key, @api_key)
      should "change password" do
        refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
      end
    end
    context "with reset_api_key false and valid password" do
      setup do
        put :update, params: {
          user_id: @user.id,
          token: @user.confirmation_token,
          password_reset: { reset_api_key: "false", password: PasswordHelpers::SECURE_TEST_PASSWORD }
        }
      end
      should respond_with :found
      should "not change api_key" do
        assert_equal(@user.reload.api_key, @api_key)
      end
      should "change password" do
        refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
    end
    context "with reset_api_key and valid password" do
      setup do
        put :update, params: {
          user_id: @user.id,
          token: @user.confirmation_token,
          password_reset: { reset_api_key: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD }
        }
      end
      should respond_with :found
      should "change api_key" do
        refute_equal(@user.reload.api_key, @api_key)
      end
      should "change password" do
        refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
      end
      should "not delete new api key" do
        refute_predicate @new_api_key.reload, :destroyed?
        refute_empty @user.reload.api_keys
      end
    end
    context "with reset_api_key and reset_api_keys and valid password" do
      setup do
        put :update, params: {
          user_id: @user.id,
          token: @user.confirmation_token,
          password_reset: { reset_api_key: "true", reset_api_keys: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD }
        }
      end
      should respond_with :found
      should "change api_key" do
        refute_equal(@user.reload.api_key, @api_key)
      end
      should "change password" do
        refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
      end
      should "expire new api key" do
        assert_empty @user.reload.api_keys.unexpired
        refute_empty @user.reload.api_keys.expired
    assert page.has_content? "Sign in"

    assert page.has_content?("Sign out")

Fixed code:
  before_action :redirect_to_verify, if: -> { current_user_is_owner? && !verified_session_active? }
      before_action :redirect_to_verify, **opts, unless: :verified_session_active?

    def session_verified
      session[:verified_user] = current_user.id
      session[:verification] = Gemcutter::PASSWORD_VERIFICATION_EXPIRY.from_now
    end

    def verified_session_active?
      session[:verification] &&
        session[:verification] > Time.current &&
        session.fetch(:verified_user, "") == current_user.id
    end
  include SessionVerifiable
  # By default, clearance expects the token to be submitted with the password update.
  # We already invalidated the token when the user became verified by token(+mfa).
  skip_before_action :ensure_existing_user, only: %i[update]
  # Instead of the token, we now require the user to have been verified recently.
  verify_session_before only: %i[update]

      # When user doesn't have mfa, a valid token is a full "magic link" sign in.
      verified_sign_in
    if current_user.update_password password_from_password_reset_params
      current_user.reset_api_key! if reset_params[:reset_api_key] == "true"
      current_user.api_keys.expire_all! if reset_params[:reset_api_keys] == "true"
      # When the user identified by the email token submits adequate totp, they are logged in
      verified_sign_in
    # When the user identified by the email token submits verified webauthn, they are logged in
    verified_sign_in
  def verified_sign_in
    sign_in @user
    session_verified
    @user.update!(confirmation_token: nil)
    StatsD.increment "login.success"
  end


  def redirect_to_verify
    session[:redirect_uri] = verify_session_redirect_path
    redirect_to verify_session_path, alert: t("verification_expired")
  end
  include SessionVerifiable
    session_verified
             :url => user_password_path(current_user),
  <%= error_messages_for current_user %>
  verification_expired:
  verification_expired: The verification has expired. Please verify again.
  verification_expired:
  verification_expired:
  verification_expired:
  verification_expired:
  verification_expired:
  verification_expired:
  verification_expired:
      should "sign in the user" do
        assert_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "invalidate the confirmation_token" do
        assert_nil @user.reload.confirmation_token
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end

        should "sign in the user" do
          assert_predicate @controller.request.env[:clearance], :signed_in?
        end

        should "invalidate the confirmation_token" do
          assert_nil @user.reload.confirmation_token
        end


        should "not sign in the user" do
          refute_predicate @controller.request.env[:clearance], :signed_in?
        end

      should "sign in the user" do
        assert_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "invalidate the confirmation_token" do
        assert_nil @user.reload.confirmation_token
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end

      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end


    context "when not signed in" do
          password_reset: { reset_api_key: "true", reset_api_keys: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD }
      should redirect_to("the sign in page") { sign_in_path }
      should "not sign in the user" do
        refute_predicate @controller.request.env[:clearance], :signed_in?
      end
    context "when signed in" do
        sign_in_as @user
        session[:verification] = 10.minutes.from_now
        session[:verified_user] = @user.id
      teardown do
        session[:verification] = nil
        session[:verified_user] = nil
      context "with invalid password" do
        setup do
          put :update, params: {
            user_id: @user.id,
            token: @user.confirmation_token,
            password_reset: { reset_api_key: "true", password: "pass" }
          }
        end
        should respond_with :success
        should "not change api_key" do
          assert_equal(@user.reload.api_key, @api_key)
        end
        should "not change password" do
          assert_equal(@user.reload.encrypted_password, @old_encrypted_password)
        end
      context "with a valid password" do
        context "when verification has expired" do
          setup do
            travel 16.minutes do
              put :update, params: {
                user_id: @user.id,
                token: @user.confirmation_token,
                password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD }
              }
            end
          end
          should set_flash[:alert]
          should redirect_to("the verification page") { verify_session_path }
          should "not sign the user out" do
            assert_predicate @controller.request.env[:clearance], :signed_in?
          end
        end
        context "without reset_api_key" do
          setup do
            put :update, params: {
              user_id: @user.id,
              token: @user.confirmation_token,
              password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD }
            }
          end
          should respond_with :found
          should "not change api_key" do
            assert_equal(@user.reload.api_key, @api_key)
          end
          should "change password" do
            refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
          end
        end

        context "with reset_api_key false" do
          setup do
            put :update, params: {
              user_id: @user.id,
              token: @user.confirmation_token,
              password_reset: { reset_api_key: "false", password: PasswordHelpers::SECURE_TEST_PASSWORD }
            }
          end

          should respond_with :found

          should "not change api_key" do
            assert_equal(@user.reload.api_key, @api_key)
          end
          should "change password" do
            refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
          end
        end

        context "with reset_api_key" do
          setup do
            put :update, params: {
              user_id: @user.id,
              token: @user.confirmation_token,
              password_reset: { reset_api_key: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD }
            }
          end

          should respond_with :found

          should "change api_key" do
            refute_equal(@user.reload.api_key, @api_key)
          end
          should "change password" do
            refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
          end
          should "not delete new api key" do
            refute_predicate @new_api_key.reload, :destroyed?
            refute_empty @user.reload.api_keys
          end
        end

        context "with reset_api_key and reset_api_keys" do
          setup do
            put :update, params: {
              user_id: @user.id,
              token: @user.confirmation_token,
              password_reset: { reset_api_key: "true", reset_api_keys: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD }
            }
          end

          should respond_with :found

          should "change api_key" do
            refute_equal(@user.reload.api_key, @api_key)
          end
          should "change password" do
            refute_equal(@user.reload.encrypted_password, @old_encrypted_password)
          end
          should "expire new api key" do
            assert_empty @user.reload.api_keys.unexpired
            refute_empty @user.reload.api_keys.expired
          end
        end
    assert page.has_content?("Sign out")

    assert page.has_content? "Reset password"

    # try again
    fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD
    click_button "Save this password"

    assert @user.reload.authenticated? PasswordHelpers::SECURE_TEST_PASSWORD
  end

  test "resetting a password but waiting too long after token auth" do
    forgot_password_with @user.email

    visit password_reset_link

    fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD

    travel 16.minutes do
      click_button "Save this password"

      assert page.has_content? "verification has expired. Please verify again."
    end

    assert page.has_content?("Sign out")

    refute page.has_content?("Sign out")

    assert page.has_content?("Sign out")

    refute page.has_content? "Sign out"