Algorithm::encryption_key_from_decryption_key` skips both of the validations its siblings perform
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 111
- Forks
- 53
- Avg merge
- 5d 29m
- Merged PRs (30d)
- 2
Description
Present in published 0.1.0 (src/lib.rs:995) and unchanged on master at 1d535a11 (src/lib.rs:1031, crate version 0.2.0, unreleased). Line numbers below are from master.
Raising it now partly because the fix wants a breaking signature change: 0.2.0 has not shipped, so if that signature is going to move, this is the cheapest moment for it to move.
CONFORMANCE.md lines 52-53 state, under Algorithm review:
Dynamic API boundaries reject keys and ciphertexts tagged for another parameter set and reject invalid message, salt, key, and ciphertext lengths.
This method does neither. It has two visible faces, and one fix closes both.
The code
frodo-kem/src/lib.rs:1027-1036:
fn inner_encryption_key_from_decryption_key<B: Params>(
&self,
secret_key: &DecryptionKey,
) -> EncryptionKey {
let sk = DecryptionKeyRef::<B>(secret_key.value.as_slice(), PhantomData);
EncryptionKey {
algorithm: *self,
value: sk.public_key().to_vec(),
}
}
Compared with inner_encapsulate_with_rng, eleven lines further down at frodo-kem/src/lib.rs:1558-1565:
fn inner_encapsulate_with_rng<K: Kem, R: CryptoRng + ?Sized>(
&self,
encryption_key: &EncryptionKey,
rng: &mut R,
) -> FrodoResult<(Ciphertext, SharedSecret)> {
if encryption_key.algorithm != *self {
return Err(Error::AlgorithmMismatch);
}
let pk = EncryptionKeyRef::from_slice(encryption_key.value.as_slice())?;
The sibling checks the tag and goes through the length-checked from_slice. The method above constructs DecryptionKeyRef::<B> by direct tuple construction, which bypasses from_slice entirely, and never compares secret_key.algorithm to *self. inner_decapsulate (frodo-kem/src/lib.rs:1643) checks both tags too.
Face 1 — panic
DecryptionKeyRef::public_key (frodo-kem/src/hazmat/models.rs:454-455) slices using the receiving parameter set's lengths over the supplied key's bytes:
pub fn public_key(&self) -> &[u8] {
&self.0[P::SHARED_SECRET_LENGTH..P::SHARED_SECRET_LENGTH + P::PUBLIC_KEY_LENGTH]
}
When the receiving set's PUBLIC_KEY_LENGTH exceeds the supplied key's length, that range is out of bounds. A correctly-tagged 19888-byte FrodoKEM-640-AES decryption key handed to any of the four 1344 algorithms panics with range end index 21552 out of range for slice of length 19888 — measured on 96 of 96 right-length buffers at all four 1344 parameter sets.
let dk = Algorithm::FrodoKem640Aes.decryption_key_from_bytes(&bytes_19888)?;
let _ = Algorithm::FrodoKem1344Shake.encryption_key_from_decryption_key(&dk); // panics
Face 2 — silent wrong answer
When the supplied key is long enough for the receiving set's public-key window, there is no panic. The method returns a well-formed, correct-length EncryptionKey tagged with the receiving algorithm, built from bytes that are not that algorithm's public key. Ciphertexts produced under it decapsulate to nothing anyone can use.
The signature is -> EncryptionKey, not -> FrodoResult<EncryptionKey>, so the method has no way to report the mismatch even if it detected one.
Suggested fix
Matching the siblings:
if secret_key.algorithm != *self {
return Err(Error::AlgorithmMismatch);
}
let sk = DecryptionKeyRef::<B>::from_slice(secret_key.value.as_slice())?;
That needs the public method's return type to become FrodoResult<EncryptionKey>, which is a breaking change — hence raising it as an issue rather than sending a patch, since the shape of the fix is your call.
The infallible impl From<&DecryptionKey> for EncryptionKey (frodo-kem/src/lib.rs:311-317) is unaffected: it reads secret_key.algorithm, so it is always self-consistent and cannot reach either face.
On reachability, stated carefully
Neither face is reachable through the idiomatic From path. Both require an application that pairs a DecryptionKey obtained from one source with an Algorithm value obtained from another.
That shape is plausible rather than observed: the serde impl (frodo-kem/src/lib.rs:130-242, applied at frodo-kem/src/lib.rs:386) puts the algorithm tag on the wire as the first byte of the binary form, and the deserialiser routes on it, so an application that deserialises a DecryptionKey and then calls the method with its own configured Algorithm would be crashable by whoever supplied those bytes. I did not find such an application; I am describing an API shape that permits one.
Worth noting separately: 19888 all-zero bytes are accepted as a FrodoKEM-640-AES decryption key, so no genuine key material is needed to reach the panic.
This is an API-contract and availability issue, not a vulnerability — no key recovery, no plaintext recovery, and the panic is a Rust bounds check doing its job rather than an out-of-bounds read. I am not filing it as a security advisory.
Method
Found with our own PQC test harness, and this issue was drafted with AI assistance — both stated up front as a matter of policy. Every claim above was reproduced by execution, then re-checked independently by three reviewers who did not share harness code. Two of the three tempered the original reachability wording, and this text reflects the tempered version.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in frodo-kem/src/lib.rs at inner_encryption_key_from_decryption_key and compare it with inner_encapsulate_with_rng and inner_decapsulate; inspect DecryptionKeyRef::from_slice and public_key in frodo-kem/src/hazmat/models.rs. Done means mismatched algorithms and invalid key lengths return errors instead of producing a key or panicking, while the unaffected From implementation remains consistent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cryptography, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100