search

auth0 / node-samlp

SAML Protocol support for node (only IdP for now)
MIT License
136 stars 117 forks source link

Math.random() is not cryptographically secure #111

Closed gen0cide closed 3 years ago

gen0cide commented 3 years ago

https://github.com/auth0/node-samlp/blob/cb5adc4557f05a72a12d4f9573615767f4ed7c5b/lib/utils.js#L132

This function is used to generate unique UIDs throughout the SAML library. Given that SAML deals with authentication, it seems like poor practice to not generate random UIDs in a cryptographically secure manner.

Reference: https://nodejs.org/api/crypto.html#crypto_crypto_randomint_min_max_callback Guidance: https://gist.github.com/joepie91/7105003c3b26e65efcea63f3db82dfba

tomauth0 commented 3 years ago

Hi @gen0cide - I'm closing this ticket off as the generateUniqueID fn has been updated in #116 to use https://github.com/auth0/id-generator internally - the implementation of that lib mimics the nodejs randomInt fn you've referenced.

This change is released in v5.0.1.

Thanks, Tom