Use of cryptographically weak random number generator for password generation in umbraco/umbraco-cms

Valid

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.

We are processing your report and will contact the umbraco/umbraco-cms team within 24 hours. 6 months ago
We have contacted a member of the umbraco/umbraco-cms team and are waiting to hear back 6 months ago
We have sent a follow up to the umbraco/umbraco-cms team. We will try again in 7 days. 6 months ago
umbraco/umbraco-cms maintainer modified the report
6 months ago
umbraco/umbraco-cms maintainer has acknowledged this report 6 months ago
umbraco/umbraco-cms maintainer
6 months ago

Maintainer


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.

Michael Rowley
6 months ago

Researcher


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

umbraco/umbraco-cms maintainer
6 months ago

Maintainer


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.

Michael Rowley
6 months ago

Researcher


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.

umbraco/umbraco-cms maintainer
6 months ago

Maintainer


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.

Michael Rowley
6 months ago

Researcher


Okay, that sounds good to me!

umbraco/umbraco-cms maintainer
a month ago

Maintainer


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

umbraco/umbraco-cms maintainer validated this vulnerability a month ago
Michael Rowley has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
umbraco/umbraco-cms maintainer confirmed that a fix has been merged on 17a9c8 a month ago
The fix bounty has been dropped
Michael Rowley
a month ago

Researcher


That's great, thanks for getting this fixed!

to join this conversation