Finding
Two correctness-critical paths in crates/koinon/src/tamper_log.rs have no test coverage:
- The
EntryTooLarge allocation guard in decode_entry. The function reads a 4-byte little-endian length prefix and rejects payloads exceeding MAX_ENTRY_BYTES before allocating the read buffer. No test feeds an oversized length prefix and asserts the guard fires.
- The CBOR roundtrip of
LogEntryKind::VaultMutation. Every other LogEntryKind variant has a roundtrip test; VaultMutation — the variant recording credential vault lifecycle events — has none.
Evidence
crates/koinon/src/tamper_log.rs:228 — the allocation guard:
if payload_len > MAX_ENTRY_BYTES {
return Err(TamperLogError::EntryTooLarge {
size: payload_len,
max: MAX_ENTRY_BYTES,
});
}
The buffer is allocated from the attacker-controlled length immediately after the guard:
let mut cbor_bytes = vec![0u8; usize::try_from(payload_len).unwrap_or_default()];
crates/koinon/src/tamper_log.rs:158 — the untested variant:
VaultMutation {
/// Human-readable credential name affected by the mutation.
credential_name: CompactString,
/// Mutation operation, e.g. `"add"`, `"rotate"`, `"revoke"`, or `"remove"`.
operation: CompactString,
},
crates/koinon/src/tamper_log_tests.rs — roundtrip tests exist for five sibling variants but not VaultMutation, and no test references EntryTooLarge:
338: fn cbor_roundtrip_signal_observed() {
351: fn cbor_roundtrip_entity_created() {
363: fn cbor_roundtrip_config_changed() {
375: fn cbor_roundtrip_alert_raised() {
387: fn cbor_roundtrip_action_taken() {
A grep of the test file for EntryTooLarge, entry_too_large, VaultMutation, and vault_mutation returns no matches.
Why this matters
Under a counter-surveillance threat model the tamper log is the integrity record an adversary most wants to corrupt or weaponize:
- The
EntryTooLarge guard is the only thing standing between a crafted or corrupted length field and a pre-validation allocation. A wire frame whose 4-byte prefix is 0xFFFFFFFF would, absent the guard, drive vec![0u8; ...] to attempt a ~4 GiB allocation before the subsequent read_exact fails — a denial-of-service primitive triggerable by feeding decode_entry a single malformed frame. With no regression test, a future refactor of decode_entry could silently drop the guard and reopen this vector undetected.
VaultMutation records the highest-sensitivity events in the log: credential add/rotate/revoke/remove. A CBOR schema drift that breaks this variant's encode/decode roundtrip would silently corrupt the audit trail for exactly the operations whose integrity matters most, and the corruption would pass every existing test because that variant is never exercised.
Desired correction
- Add a test that builds a wire frame with a length prefix of
0xFFFFFFFF (4 bytes 0xFF) followed by sufficient bytes, calls decode_entry, and asserts Err(TamperLogError::EntryTooLarge { size, max }) with the expected max == MAX_ENTRY_BYTES.
- Add a
cbor_roundtrip_vault_mutation test mirroring the existing sibling roundtrip tests: construct a LogEntry with LogEntryKind::VaultMutation, encode_entry then decode_entry, and assert equality of the decoded entry.
Done when: both tests exist and pass, removing the EntryTooLarge guard at line 228 makes the first test fail, and altering the VaultMutation CBOR shape makes the second test fail.
Finding
Two correctness-critical paths in
crates/koinon/src/tamper_log.rshave no test coverage:EntryTooLargeallocation guard indecode_entry. The function reads a 4-byte little-endian length prefix and rejects payloads exceedingMAX_ENTRY_BYTESbefore allocating the read buffer. No test feeds an oversized length prefix and asserts the guard fires.LogEntryKind::VaultMutation. Every otherLogEntryKindvariant has a roundtrip test;VaultMutation— the variant recording credential vault lifecycle events — has none.Evidence
crates/koinon/src/tamper_log.rs:228— the allocation guard:The buffer is allocated from the attacker-controlled length immediately after the guard:
crates/koinon/src/tamper_log.rs:158— the untested variant:crates/koinon/src/tamper_log_tests.rs— roundtrip tests exist for five sibling variants but notVaultMutation, and no test referencesEntryTooLarge:A grep of the test file for
EntryTooLarge,entry_too_large,VaultMutation, andvault_mutationreturns no matches.Why this matters
Under a counter-surveillance threat model the tamper log is the integrity record an adversary most wants to corrupt or weaponize:
EntryTooLargeguard is the only thing standing between a crafted or corrupted length field and a pre-validation allocation. A wire frame whose 4-byte prefix is0xFFFFFFFFwould, absent the guard, drivevec![0u8; ...]to attempt a ~4 GiB allocation before the subsequentread_exactfails — a denial-of-service primitive triggerable by feedingdecode_entrya single malformed frame. With no regression test, a future refactor ofdecode_entrycould silently drop the guard and reopen this vector undetected.VaultMutationrecords the highest-sensitivity events in the log: credential add/rotate/revoke/remove. A CBOR schema drift that breaks this variant's encode/decode roundtrip would silently corrupt the audit trail for exactly the operations whose integrity matters most, and the corruption would pass every existing test because that variant is never exercised.Desired correction
0xFFFFFFFF(4 bytes0xFF) followed by sufficient bytes, callsdecode_entry, and assertsErr(TamperLogError::EntryTooLarge { size, max })with the expectedmax == MAX_ENTRY_BYTES.cbor_roundtrip_vault_mutationtest mirroring the existing sibling roundtrip tests: construct aLogEntrywithLogEntryKind::VaultMutation,encode_entrythendecode_entry, and assert equality of the decoded entry.Done when: both tests exist and pass, removing the
EntryTooLargeguard at line 228 makes the first test fail, and altering theVaultMutationCBOR shape makes the second test fail.