Use of cryptographically weak random number generator for password generation in umbraco/umbraco-cms
Reported on
Mar 29th 2022
Description
Umbraco has a GeneratePassword
function that is used to generate passwords that should be unpredictable, this function uses the .NET Random class which isn't cryptographically secure.
Impact
This vulnerability is capable of allowing attackers to predict generated passwords and use them to login to newly-created accounts.
References
SECURITY.md
exists
2 years ago
Thanks for sending in this report. We understand that conceptually this is not secure, but we haven't been able to create a tool that manages to actually predict future values yet.
Hi, thanks for your message - the below StackOverflow post and CodingVision article (also referenced in the initial report) provide proof-of-concepts for predicting the output of the RNG referenced in this report: https://codingvision.net/c-predict-random-number-generator-net https://stackoverflow.com/questions/39093911/can-random-next-be-predicted-from-its-outputs
Hi Michael. Thanks for the links. The problem we are having is that we are never able to get anywhere near 55 consecutive values, and the method instantiates a new Random() each time the GeneratePassword() method is called. The password generation process starts with calling PasswordStore.GeneratePassword(int length, int numberOfNonAlphanumericCharacters), which uses RNGCryptoServiceProvider.
I agree that even if it's just for parts of the password generation, the use of random.next() is a bad idea, but we still have not been able to predict values using the approach in the links provided.
Ah I hadn't noticed that the majority of the generated password uses the RNGCryptoServiceProvider class so I'd agree that this isn't really an exploitable issue but with a new instance of Random being created with each password generation, it may be possible to derive which 'requirement' characters would be appended to the password by just knowing when the password would be generated (Random defaults to using system time as a seed so if I generated four random numbers with a new Random instance at the same second as another user generates a password, those random numbers that I have generated should correlate to the RequireDigit, RequireLowercase, RequireUppercase, and RequireNonLetterOrDigit characters).
Thanks for taking the time to clarify about the context of the code, It's up to you whether you'd like the report to be closed since the security impact is minimal.
I will discuss it with the team and decide how we handle it. I am leaning towards confirming it as a vulnerability with low severity. Even if it's very unlikely to be exploitable, I think the fact that some characters can theoretically be guessed is enough for us to add it to our backlog and fix it for upcoming versions. I'll get back to you as soon as I have an update.
This has been fixed and merged into the Umbraco 10.2 scheduled to be released for the 8th of September - https://github.com/umbraco/Umbraco-CMS/pull/12759