Missing Cryptographic Step in star7th/showdoc

Valid

Reported on

Aug 2nd 2021


✍️ Description

The referenced code contains a hard-coded salt that is used for all passwords, ideally - a unique salt should be generated for each password and then would be stored alongside it as oppose to the constant one that is used for all passwords in the showdoc repository.

🕵️‍♂️ Proof of Concept

  • Execute the following PHP code:
<?php
for ( $i = 0; $i < 30; $i++ ) {
    $password = random_int( 0, 1 ) == 1 ? "alpha" : "delta";
    echo md5( $password."576hbgh6" )."</br>";
}
?>
  • Notice how each ciphertext is 7c062e5f87a120c6d6a27ac8bd770899 or 011b22021e8583bdfe77ac6d9b525a16 - this shows that, with a constant, non-randomized hash - ciphertexts of the same input will result in the same output - therefore rainbow tables can be generated with the hardcoded salt in mind.

💥 Impact

This vulnerability is capable of allowing attackers to generate database-effective rainbow tables.

Ziding Zhang
4 months ago

Admin


Hey Michael, I've also emailed them regarding this report too. Great work!

We have contacted a member of the star7th/showdoc team and are waiting to hear back 4 months ago
star7th/showdoc maintainer confirmed that a fix has been merged on 4b962c 4 months ago
The fix bounty has been dropped
Michael Rowley
4 months ago

Researcher


@admin This is my fix from CVE-2021-3678 - am I still eligible for the fix bounty?

star7th/showdoc maintainer
4 months ago

I made a mistake. “https://github.com/star7th/showdoc/commit/4b962c1740311e0d46775023b6acba39ad60e370 " Should belong to " https://huntr.dev/bounties/f9a9defd-29ea-4442-b692-ff1512813de4 " Fix this problem.

For this problem, there will be trouble in repairing. Because if you change a way to verify your password, it may cause the old user to be unable to log in. At present, my restrictive measures to prevent brute force cracking are: when the password of the same user is wrong more than a certain number of times, the verification code will appear. Hackers cannot simply enumerate to crack user login. I don't know if this can improve security.

Jamie Slome
4 months ago

Admin


I have correctly represented the correct commit SHA on the other report and have prepared CVE-2021-3678 for publishing.

In both circumstances, we cannot reward the fix bounty as the fix submission did not occur through the platform.

Jamie Slome
4 months ago

Admin


Preparing to be published.

Michael Rowley
4 months ago

Researcher


In both circumstances, we cannot reward the fix bounty as the fix submission did not occur through the platform.

For future reports, is there something that I can do in instances where the maintainer or another researcher has submitted a faulty patch so that I can submit a working one and receive a fix bounty (as for this one, I couldn't use the 'Submit Patch' button as the maintainer had marked the faulty patch as the fix)

Jamie Slome
4 months ago

Admin


I understand, would you be able to create an issue on our public GitHub repository, and we will get this feature request tracked for you!

Create GitHub Issue

Michael Rowley
4 months ago

Researcher


"if you change a way to verify your password, it may cause the old user to be unable to log in" I'm not too sure what you mean, surely old users' 'secret keys' would only be used for their log-in session so restarting their showdoc instance would get rid of any incompatibilities?

I can't figure out exactly where these secret keys are used as the JavaScript file pointed to by Github seems obfuscated.

star7th/showdoc maintainer
4 months ago

old users' 'secret keys' are stored in the database (/Sqlite/showdoc.db.php) . Each time a user logs in, the program tries to find thesesecret keys

https://github.com/star7th/showdoc/blob/fd1740234a12804b45af9cac3563567d83ba414d/server/Application/Home/Model/UserModel.class.php#L46

Michael Rowley
4 months ago

Researcher


Ah, in that case; it might be worth suggesting that users wipe their database when updating to the new version so that their databases are only populated with the new secure secret keys.

star7th/showdoc maintainer
4 months ago

In fact, I knew this problem a long time ago. However, after weighing, I think the problem is not big, and I think it is more priority to be compatible with history. I think so: in the current situation, unless hackers directly get the database and the encryption string, they can reverse restore the original password with things such as rainbow table. However, the premise is that the hacker can get the database. So I think the risk is still very low. Unless there is really a way I don't know to get to the database.

Michael Rowley
4 months ago

Researcher


I agree that an attacker would need access to the database to provide any realistic impact but assuming that nobody would have unauthorized access to the Showdoc database seems entirely unrealistic as, if that was true; hashes and salts altogether would be useless as nobody would ever access the database in order to try out the passwords. The showdoc program is unlikely to be the only software running on a server (and those other programs could be used to compromise the server/system) so wherever possible, steps should ideally be taken to ensure that even if the server is compromised, end-user data is unable to be exfiltrated as plaintext or easily brute-forced.

Michael Rowley
4 months ago

Researcher


This issue is still persistent in other areas of the codebase and will require a salting system to be implemented to be properly fixed. https://github.com/star7th/showdoc/blob/d1e5a05ebaaaaba5b63cf339e6c4951e60b58a49/server/Application/Home/Model/UserModel.class.php#L19-L28 For another example.

star7th
4 months ago

Maintainer


Well, I may introduce a salt mechanism to fix this problem later. However, in view of the small impact of this problem, it may be done for some time.