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

The OTP secrets are generated with only 10 bytes of entropy #220

Closed
disconnect3d opened this issue Jul 27, 2022 · 0 comments · Fixed by #276
Closed

The OTP secrets are generated with only 10 bytes of entropy #220

disconnect3d opened this issue Jul 27, 2022 · 0 comments · Fixed by #276

Comments

@disconnect3d
Copy link

Hey,

This is potentially a security vulnerability. I am reporting it here since you neither have a security policy in your repository nor your website uses the https://securitytxt.org/ standard (I would expect https://nhost.io/security.txt to provide contact information). This is also not of a huge impact, but still.

This repo depends on [email protected] for OTP secrets generation which uses only 10 bytes of entropy for the OTP secret generation. This may allow someone who knows a few OTP tokens and the time they were generated to brute force the random value used to generate the secret and then predict future generated OTP tokens.

The hasura-auth uses the otplib for secret generation in here:

const totpSecret = authenticator.generateSecret();
const otpAuth = authenticator.keyuri(
userId,
ENV.AUTH_MFA_TOTP_ISSUER,
totpSecret
);
await gqlSdk.updateUser({
id: userId,
user: {
totpSecret,
},
});
const imageUrl = await createQR(otpAuth);
return res.send({ imageUrl, totpSecret });

The totpSecret is generated from 10 bytes of randomness since this is the default number of bytes the [email protected] uses under the hood. This can be seen here which uses this method under the hood.

By the way, this behavior is also not compliant with some RFCs that recommend to use at least 160 bits of randomness.

Also please note that previous versions of otplib used 20 bytes of randomness instead of 10 for secret generation. I have no idea why this was changed in 12.0.0 to just 10 bytes.

This issue is also reported on the upstream otplib repository here: yeojz/otplib#671

To fix the issue, you can simply use e.g. authenticator.generateSecret(32); which would provide 32 bytes (256 bits) instead of the default (10) bytes of randomness for secret generation.

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

Successfully merging a pull request may close this issue.

2 participants