On Thursday, 04/16/2021, I was made aware of vulnerabilities present in croc, a file-transfer utility that I have been maintaining for several years (for more about croc, see my previous post).

The vulnerabilities were discovered and disclosed by Aaron Kaiser of the RedRocket CTF team. What Aaron showed me was that on croc (version 8) an exploit could be created to produce a rogue receiver that could backdoor and then bruteforce the SPAKE2 algorithm and receive a file. This attack, though serious, is not covert, as the sender would notice their file being transferred to someplace other than the true receiver. The technical details of this attack are very interesting, thoroughly documented, and cleverly executed and can be found in RedRocket’s blog post about croc’s security flaws.

Luckily, this critical vulnerability seems to have not been exploited. Users that utilized the public relay could have been exploited, but they would have been made aware immediately by recognizing that the sender transferred a file while the receiver never received a file. So far, I have no reports of any users experiencing this behavior. (If you have experienced this behavior, please let me know ASAP via keybase). Also, fortunately, the exploit was found and raised to me privately so I don’t believe it was ever used in nefarious ways. To my knowledge, the exploit found would not work on private relays since it requires to have the rogue recipient first bind itself to the relay (which it cannot do if a relay is set to private).

RedRocket wrote a detailed analysis of croc and outlined several points of failure within it, including this most serious exploit mentioned above as well as a few others. This post will address these vulnerabilities and the changes I made to croc to solve them. These changes are reflected in croc, version 9.

Fixing vulnerabilities and releasing croc version 9

The following is a point-by-point discussion of the flaws discovered in RedRocket’s blog post and the fixes I developed to patch them. To start, though, I want to acknowledge that I am grateful to RedRocket for providing these security observations to me in such a precise and helpful way, and would urge anyone out there to support RedRocket and their CVE discoveries of open-source software. As pointed out by RedRocket:

A small bug can go a long way. It is difficult to implement crypto protocols correctly, especially when a small detail can completely break all security. Unfortunately that’s the case for most protocols. Cyptography is still scary.

I believe this to be very true, and I try to work hard that I can make sure crypto protocols work well in croc so that it remains secure.

Vulnerability in SPAKE2 implementation

As described in the RFC for SPAKE2 the public identities for both parties should be hardcoded by the application. These values should be chosen in a way where the developer can prove that it is unlikely for him to know the discrete logarithm of these values as well.

This has been fixed in pake.go for croc’s SPAKE2 implementation.

As RedRocket notes, my implementation of SPAKE2 did not enforce a specific value for the points along the elliptic curve. In my implementation these points were chosen at random. However, what Aaron of RedRocket cleverly figured out, was that they could choose entities for the curves with known discrete logarithms so that the produced key was much weaker (and breakable through bruteforce).

As suggested by RedRocket, and as described in the forthcoming RFC for SPAKE2, the public identifies should be hardcoded and the values should be chosen in a way that I can prove its unlikely I know the discrete algorithm. So, with help from Travis Scholl, these public identities were generated using a hash of a phrase croc1 and croc2 to prove I don’t know the discrete algorithm. Here is the code to verify the points: SageMath code to generate points.

SIEC use

Unfortunately, croc uses a very unoptimized implementation of an uncommon cofactor-one curve called siec. So for our exploit to work in a reasonable time, we either had to properly implement the curve ourselves or throw more computational power at it. Out of convinience we took the latter option and spun up 10 AWS instances as bruteforce workers.

croc by default uses a less known curve called SIEC. It belongs to a class of curves called “super-isolated” and were first introduced by Travis Scholl. Being super-isolated means it is more difficult to transfer the discrete log problem (the basis of security for classical elliptic curve cryptosystems) to another curve. Hence the security of a SIEC curve does not reduce to the security of the curves around it. These curves necessarily have a small conductor (similar to the curve Bitcoin uses Secp256k1) , and usually admit low degree endomorphisms which could be used to speed up point addition using the well-known GLV/GLS method. Some believe curves with “special features” are less secure. But there are cases where more general objects are less secure, like for genus 3 curves. Because SIEC has not had the popularity of other curves, it has not received the same careful hand optimizing other curves had like secp256k1, P-256, and ec25519. But there is no reason that a similar effort could be made to significantly speed up curve operations. And for those who would rather use a standard curve, it is a minor change to make configurable.

Anyways, the RedRocket team says “unfortunately” here which I take to mean that it was unfortunate for them to have to spin up AWS computers since the siec implementation is less optimized than others - not that the curve itself is unfortunate for cryptography (which I believe is not, as outlined above).

Sender and recipient should not exchange bcrypt hash of key

The sender then answers with its public parameter YYY as well as HkbH_{kb}Hkb​, which is a bcrypt hash of the exchanged key. This is an absolutely terrible idea and already enough to brute force the exchanged key. Unfortunately, bcrypt is used for hashing, which is computationally expensive. With our resources we would need at least a few days to brute force the the passcode based on this hash….Also there should be a check implemented which ensures that Hka and Hkb are different from each other.

This has been fixed to remove any hash exchange of the secure key.

Part of the attack described above was made possible because of a superfluous check that the secure passphrase generated from SPAKE2 are correct between the two parties. I had the same hash for each party which enabled a nefarious party to skip this check by sending back the hash it received from the other.

However, since I’m using AES-GCM with the generated key, I don’t need this check at all and it has been removed from the code. If the two parties have different keys it will be obvious when it fails the decryption, and AES-GCM is able to both decrypt and authenticate the message. This also has the beneficial byproduct of being faster than the previous implementation since bcrypt is not needed anymore.

Room leaks first word of the passcode

The room should not be derived from the passcode but rather uses a separate identifier.

This has been fixed to add a “pin” that designates room so that the weak key is never used as part of the room for exchange.

All generated code-phrases are preceded by a 4-digit pin number that is used to identify the “room” for the transfer to take place. The rest of the code-phrase is used for the shared secret to generate the secure key. This will ensure there is no information about the secure passphrase generation passed between the two parties.

Pathtraversal issues

The sender should not be able to control the path of the file. The recipient should just take the filename from the sender and should save the file to the current working directory.

This has been fixed to prompt overwrites.

In every version of croc the sender is only able to control the path of the file within the recipient’s current working directory. However, the ability to be informed of overwriting files is useful and I have now added it.

To be clear, there are two modes of file transmission in croc:

1. Sender sends a single file. In this case, the recipient will save the file to its current directory no matter where the file originated from the sender. This has been the default behavior for all versions of croc.
2. Sender sends a folder. In this case, the sender cannot send a folder that is not in the sender’s working directory. The recipient will receive the folder and save it and everything in it into the recipient’s working directory.

Previous versions of croc would automatically overwrite files if they were not the same (but only ever in the working directory of the recipient). I changed the default behavior to prompt for overwriting, which can be turned off with -overwrite. The limitation to receive files in the current working directory is not alterable.

Protocol sequence is not enforced

All messages between the recipient and sender are handelt by the processMessage function in croc.go. This function does not enforce the sequence of the messages. If an encryption key is set incoming messages are decrypted. Otherwise the messages are processed without decryption. This allows an attacker to start the protocol with a external ip message and omit the key exchange to use the protocol without encryption. The only thing preventing the sender from sending the file unencrypted is the fact that the go crypto library will throw an error while encrypting data with a nil key and that will result in a panic.

This has been fixed.

In croc version 8, only the PAKE communication is unencrypted and that all other communication requires encryption. So in the now version 9, I have fixed it so that only PAKE communication is explicitly allowed to be unencrypted, while all other messages must be encrypted or it will be rejected with error.

Send-to-relay security

This sender-to-relay connection is “secured” with a SPAKE2 key exchange. Strangely this connection uses the hardcoded passcode pass123 and therefore does not provide security against an active attacker. Although somewhat nasty, we won’t focus on this bug here because confidentiality and authenticity are not crucial for this step.

In this case, the pass123 is simplistic because it is a public relay and it only exchanges the name of the room during the transmissions with the ephemeral key generated with this simple passphrase to encrypt “over-the-wire”. After talking with RedRocket I’ve realized that better security would be to additionally authenticate the public server by generating a keypair and binding the default croc client with the public key so that the public relay (and private) can be authenticated and not just encrypted via a common passphrase. This is work in progress, soon to be released in v10 (probably next week).