From db68c07467323004ca0c69ee76a567ecef91580d Mon Sep 17 00:00:00 2001 From: Aleksandr Menenkov <119082209+Retengart@users.noreply.github.com> Date: Tue, 5 Aug 2025 13:50:16 +0300 Subject: [PATCH] add: more rand.Read() checks --- .gitignore | 2 +- rec.md | 35 +++++++++++++++++++++++++++++++++++ src/Picocrypt.go | 16 ++++++++-------- 3 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 rec.md diff --git a/.gitignore b/.gitignore index e06e4dc..d9321e4 100644 --- a/.gitignore +++ b/.gitignore @@ -20,5 +20,5 @@ # Go workspace file go.work go.work.sum - +.idea/ TODO diff --git a/rec.md b/rec.md new file mode 100644 index 0000000..e039bc6 --- /dev/null +++ b/rec.md @@ -0,0 +1,35 @@ +### Summary of Findings + +1. **PCC-001** + - **Type:** Design decision + - **Description:** Data encrypted with Picocrypt is stored in a custom file format, including a header which is unauthenticated. As a result, any changes made by an attacker would go undetected. + - **Threat level:** Low + +2. **PCC-004** + - **Type:** Cryptographic implementation + - **Description:** Picocrypt offers both encryption and authentication. When a user wants to decrypt a volume, it starts with decrypting and only verifies the signature afterwards. As a result a user may unknowingly use their private key on attacker-controlled material. + - **Threat level:** Low + +3. **PCC-006** + - **Type:** Design decision + - **Description:** Picocrypt uses a memory-hard key derivation function to limit brute force attacks on the password. The output is then hashed using SHA3-512 and stored in the header, to verify the key before attempting to decrypt. As a result there are two algorithms that can be attacked individually which would both lead to the key. + - **Threat level:** N/A + +### Summary of Recommendations + +1. **PCC-001** + - **Type:** Design decision + - **Recommendation:** + - Authenticate data before processing. Since part of the header is required for key derivation, this is not possible. Adding authentication to the header would allow users to be informed if the header was tampered with (after key derivation, but before decryption). + +2. **PCC-004** + - **Type:** Cryptographic implementation + - **Recommendation:** + - Authenticate the ciphertext before decrypting it. + +3. **PCC-006** + - **Type:** Design decision + - **Recommendation:** + - Do not store the hash of the key in the header. + - Verify the signature before attempting to decrypt the volume. + - Consider replacing the hash with a MAC of the header. \ No newline at end of file diff --git a/src/Picocrypt.go b/src/Picocrypt.go index 0e81efd..22d1635 100644 --- a/src/Picocrypt.go +++ b/src/Picocrypt.go @@ -1637,17 +1637,17 @@ func work() { _, errs[3] = fout.Write(rsEncode(rs5, flags)) // Fill values with Go's CSPRNG - if _, err := rand.Read(salt); err != nil { - panic(err) + if n, err := rand.Read(salt); err != nil || n != 16 { + panic(errors.New("failed to read 16 bytes from crypto/rand")) } - if _, err := rand.Read(hkdfSalt); err != nil { - panic(err) + if n, err := rand.Read(hkdfSalt); err != nil || n != 32 { + panic(errors.New("failed to read 32 bytes from crypto/rand")) } - if _, err := rand.Read(serpentIV); err != nil { - panic(err) + if n, err := rand.Read(serpentIV); err != nil || n != 16 { + panic(errors.New("failed to read 16 bytes from crypto/rand")) } - if _, err := rand.Read(nonce); err != nil { - panic(err) + if n, err := rand.Read(nonce); err != nil || n != 24 { + panic(errors.New("failed to read 24 bytes from crypto/rand")) } if bytes.Equal(salt, make([]byte, 16)) { panic(errors.New("fatal crypto/rand error"))