CuriousLearner / django-phone-verify

A Django app to support phone number verification using security code / One-Time-Password (OTP) sent via SMS.
https://www.sanyamkhurana.com/django-phone-verify/
GNU General Public License v3.0
258 stars 61 forks source link

Security Code is returned in Session Token? #47

Closed rkrueger11 closed 4 years ago

rkrueger11 commented 4 years ago

I'm concerned about the security of this application. (or maybe I'm just confused?)

Why do you include the Security Code in the payload of the session token when responding to the register endpoint? This is not encrypted and can easily be decoded.

The end user would not need access to the phone number in order to type in the security code and "verify" they own the phone number.

CuriousLearner commented 4 years ago

Hi @rkrueger11

Why do you include the Security Code in the payload of the session token when responding to the register endpoint? This is not encrypted and can easily be decoded.

Every time a new random security code is generated we generate a session_token in order to identify that it is the same device that requested a verification.

For example, treat a case when you want to verify the user's phone number before even creating a user account for them. In this case, we want to ensure that the device that triggered the initial request for registration is the same device that is verifying the phone number.

Now, coming to your concern about This is not encrypted and can easily be decoded.: No, this is not true. session_token is a JWT that is encrypted with your Django's secret key. So, as long as you protect your secret key, the session_code won't be harmful.

If you expose your secret key, then even the user's information and all of your secret creds are at a risk.

Does this answer your query? Do you have any more questions/concerns?

rkrueger11 commented 4 years ago

@CuriousLearner Using this example, the JWT is not encrypted, but signed instead. JWT's can be encrypted.. but are usually not.

https://jwt.io/introduction/

CuriousLearner commented 4 years ago

Ah, yes, you're right. The session_code are signed JWT tokens.

rkrueger11 commented 4 years ago

So then, if I wanted to "verify" a phone number, I could decode the JWT to determine the security code without access to the phone number at all.

The security code should be removed from immediately. I don't follow your use case which would supposedly require it to be passed back to the client.

CuriousLearner commented 4 years ago

Sorry, but can you help me with elaborating your statement:

I could decode the JWT to determine the security code without access to the phone number at all.

rkrueger11 commented 4 years ago

Sure.

If I wanted to "verify" my account using your phone number, I could:

  1. Sign in with your phone number.
  2. You would get a text, but I would never need to see that text.
  3. I would take the session_code returned by the /register endpoint and decode it. (Could just past it in to the jwt.io site I linked to above.)
  4. This would give me the security code, which then I would use to hit the /verify endpoint.
  5. I would have successfully "verified" your phone number without your knowledge.

This is a fundamental security flaw. As such, by using this package you can not guarantee that you are verifying any phone numbers and that the user actually has access to the phone numbers they are entering.

CuriousLearner commented 4 years ago

Oh, You're right, Ryan!

Thank you so much for reporting this.

I'm sorry. I've missed this for so long. Would you like to propose a patch? If not, I'll get this done next Tuesday and make a release.