I was using BoxCryptor when they first launched for some of my customers. With their "Version 2.0" they flushed their software down the toilet: Horrible implementation. Horrible performance. Horrible bugs.
Switch to Cryptomator as this is more "future proof" from my pov. I saddens me to see that Secomba just didn't got it any right after the "update". Even after so many years.
Shoutout to the OpenLab and the rest of the Auxburg guys <3
TL;DR: this code should've never passed audit. I've found numerous problems, but I'll focus on the attack that lets someone successfully manipulate a ciphertext and have it successfully decrypt as something else. While the audit report says "Boxcryptor is not enforcing integrity [of ciphertexts]," this attack can let an adversary decrypt a (short) ciphertext, given a padding oracle. This company should've never rolled their own crypto in response to Authenticated Encryption, which has been solved, if you just use a pre-existing library.
I'm surprised that this code has a "successful" audit. The cryptography _protocol_ that is implemented in the linked github repo (https://github.com/secomba/boxcryptor-single-file-decryptor) has several flaws (in addition to having some bad code practices that I'll skip over since this repo is supposed to only document the encryption protocols).
First, the authors' problems appear to stem from their choice to manually implement an unusual (and inefficient) construction of the Authenticated Encryption primitive. Authenticated Encryption is the most common crypto primitive that people when they say "they want encryption." It's placed front-and-center in libsodium, ring, mundane, tink, monocypher, and every modern cryptography library that I've seen, since it is such a common operation. Modern Authenticated Encryption constructions include: AES-GCM, (X)ChaCha20-Poly1305. While there exist others, the industry has converged on these two as the standard.
These block cipher modes did not emerge for no reason. The cryptography community has steadily iterated on what the default should be when somebody asks, "how can I encrypt my file." We've arrived at these constructions, in particular, because previous constructions have had security flaws.
The authors of this repo have chosen to use the following protocol (for decryptDataPBKDF2):
1. Derive two keys (KE, KH) from a password string using PBKDF2. KE is the encryption key used with AES, and KH is an HMAC key (used for multiple purposes, which is problematic).
2. Read the AES-CBC initialization vector, IV. Read the AES-CBC ciphertext, C.
3. Check HMAC(KH, C) == the tag in the file.
4. Output AES-CBC-DECRYPT(KE, IV, C) with PKCS#5 padding.
The core of the problem is that, while they use the HMAC to check that the ciphertext is authentic (which is a bit odd, given that they seem to claim that authenticity shouldn't matter), they never check that the IV is authentic (it's never computed in the HMAC).
The way that AES-CBC decryption works, for first 16-bytes of the decryption is AES-BLOCK-DECRYPT(KE, first 16 bytes of C) XOR IV. As a result, if the IV isn't authenticated (which it's not), then any bit that the attacker flips in the IV will flip the corresponding bit in the ciphertext. Because PKCS#5 padding is used, given a padding oracle, an adversary could decrypt messages under 16 bytes in length.
The moral of the story is DO NOT ROLL YOUR OWN CRYPTO! Rolling your own crypto can be fun and educational and informative, but DON'T DEPLOY IT!
Since this is a .net core project, I am a little puzzled as to why they didn't use the .net AES-GCM classes. It's part of .net core 3.1. There should be no reason to use AES-CBC mode in a newer application.
Switch to Cryptomator as this is more "future proof" from my pov. I saddens me to see that Secomba just didn't got it any right after the "update". Even after so many years.
Shoutout to the OpenLab and the rest of the Auxburg guys <3