Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A consumed otp becomes valid again within the drift period #106

Closed
mnipper opened this issue Mar 7, 2017 · 9 comments
Closed

A consumed otp becomes valid again within the drift period #106

mnipper opened this issue Mar 7, 2017 · 9 comments

Comments

@mnipper
Copy link

mnipper commented Mar 7, 2017

If an otp is consumed, it becomes invalid during the otp.interval of the instantiated ROTP::TOTP object in TwoFactorAuthenticatable. otp.interval is currently always 30 seconds, since that is the default: https://github.com/mdp/rotp/blob/v3.3.0/lib/rotp/totp.rb#L10

However, during the drift period specified by otp_allowed_drift, the otp again becomes valid.

I believe this is unexpected behavior, and the intention is for an otp to not be reusable within the drift period after it has been consumed.

Here are some specs dropped in here which currently fail:

      context 'given a valid OTP used multiple times within the allowed drift' do
        let(:consumed_otp) { ROTP::TOTP.new(otp_secret).at(Time.now) }

        before do
          subject.validate_and_consume_otp!(consumed_otp)
        end

        context 'after the otp interval' do
          before do
            Timecop.travel(subject.otp.interval.seconds.from_now)
          end

          # This currently fails!
          it 'fails to validate' do
            expect(subject.validate_and_consume_otp!(consumed_otp)).to be false
          end
        end
      end
mo-zo added a commit to mo-zo/devise-two-factor that referenced this issue Mar 31, 2017
A consumed otp becomes valid again once per otp.interval until it
expires due to the drift period (otp_allowed_drift)

fixes: devise-two-factor#106
mo-zo added a commit to mo-zo/devise-two-factor that referenced this issue Mar 31, 2017
A consumed otp becomes valid again once per otp.interval until it
expires due to the drift period (otp_allowed_drift)

fixes: devise-two-factor#106
@asia653
Copy link

asia653 commented Aug 16, 2018

Can we close this issue because of the merged PR?

@sadgb
Copy link

sadgb commented Apr 4, 2019

not fixed in master

@alexanderheckel
Copy link

Hi, any plans to merge / fix the master branch?

@oliverrauscher
Copy link

Hello! I really appreciate this great gem!

Is there any chance that this critical fix will get merged into the master branch soon?

@alexanderheckel
Copy link

Hi, any updates on this?

1 similar comment
@alexanderheckel
Copy link

Hi, any updates on this?

@edwinw6
Copy link

edwinw6 commented Feb 23, 2022

The issue can be mitigated by disallowing OTP drift. To do so, add Devise.otp_allowed_drift = 0 to your devise.rb file.

I'm hoping this issue will be patched soon.

@bsedat
Copy link
Member

bsedat commented Apr 6, 2022

Thank you, this is fixed now in the published version 4.0.2.

@bsedat bsedat closed this as completed Apr 6, 2022
@ghost
Copy link

ghost commented Apr 13, 2022

Hi,
#108 remains open as a fix for this issue, so could you clarify please if 64576bb is the fixing commit for this issue, as released in 4.0.2, and therefore for https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-43177 as well?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants