diff --git a/artifacts/stpa/attack-scenarios.yaml b/artifacts/stpa/attack-scenarios.yaml index 670e96e..e8a2ab1 100644 --- a/artifacts/stpa/attack-scenarios.yaml +++ b/artifacts/stpa/attack-scenarios.yaml @@ -948,6 +948,55 @@ artifacts: - type: executed-by target: TA-3 + - id: AS-37 + type: attack-scenario + title: Cert-chain downgrade via silently-swallowed cert_count parse error + status: approved + description: > + SignatureForHashes::deserialize used `if let Ok(cert_count) = + varint::get32(...)` which silently swallowed ALL parse errors — + including WSError::ParseError from malformed cert_count bytes — + not just the EOF that signals backward compatibility with the + pre-cert-chain format. An attacker who corrupted the cert_count + bytes (e.g., 5 bytes each with MSB set; get32 consumes all 5 and + returns ParseError) could strip the certificate chain from a + cert-based signature, downgrading it to a bare-key signature + without the parser flagging the malformed input. Verified by + test_malformed_cert_count_is_rejected in + src/lib/src/signature/sig_sections.rs. Kani harness inconclusive: + CBMC OOMs symbolically exercising std::io::BufReader + Vec; + primitive-layer Kani proofs in src/lib/src/wasm_module/varint.rs + (proof_get32_no_panic, proof_get32_no_overflow) establish get32's + error behavior on malformed symbolic bytes. Related class: + wasmtime CVE-2026-27572 (panic on excessive + wasi:http/types.fields) — same family of "parser reacts wrongly + to malformed size field." Fix: replace the if-let-Ok with a + fill_buf-based peek that distinguishes clean EOF (old format + without a cert_count field) from malformed bytes (error + propagates). Discovered by Mythos delta pass on tier-5 file + sig_sections.rs. + fields: + attack-type: exploit-vulnerability + attack-feasibility: medium + elapsed-time: 3 + specialist-expertise: 3 + knowledge-of-item: 3 + window-of-opportunity: 1 + equipment: 0 + impact-safety: moderate + impact-financial: moderate + impact-operational: moderate + impact-privacy: negligible + links: + - type: exploits + target: UCA-6 + - type: exploits + target: DF-5 + - type: executed-by + target: TA-3 + - type: leads-to-hazard + target: H-9 + - id: AS-36 type: attack-scenario title: MCUboot partial-image signature via small ih_img_size diff --git a/src/lib/src/signature/sig_sections.rs b/src/lib/src/signature/sig_sections.rs index 752d35a..3c12463 100644 --- a/src/lib/src/signature/sig_sections.rs +++ b/src/lib/src/signature/sig_sections.rs @@ -83,8 +83,22 @@ impl SignatureForHashes { } let signature = varint::get_slice(&mut reader)?; - // Deserialize certificate chain (optional, for backward compatibility) - let certificate_chain = if let Ok(cert_count) = varint::get32(&mut reader) { + // Deserialize certificate chain (optional, for backward compatibility). + // + // A pre-cert-chain signature (old format) has no cert_count field — the + // byte stream ends here. A modern signature always writes at least a + // 0-byte varint for cert_count. We distinguish the two by peeking the + // reader; clean EOF is backward-compat, any other error must propagate. + // + // The previous `if let Ok(cert_count) = varint::get32(...)` pattern + // silently swallowed ALL error variants (including malformed cert_count + // bytes), downgrading cert-based signatures to bare-key signatures + // without flagging. See AS-37 / UCA-6. + let certificate_chain = if reader.fill_buf()?.is_empty() { + // Backward compat: no cert_count field at all. + None + } else { + let cert_count = varint::get32(&mut reader)?; if cert_count as usize > MAX_CERTIFICATES { debug!( "Too many certificates: {} (max: {})", @@ -113,8 +127,6 @@ impl SignatureForHashes { } else { None } - } else { - None }; Ok(Self { @@ -492,4 +504,27 @@ mod tests { // Payloads should be different (random) assert_ne!(section1.payload(), section2.payload()); } + + /// Regression test for AS-37 / UCA-6: malformed cert_count bytes must + /// propagate as an error, not be silently converted to `certificate_chain: + /// None` (which would downgrade a cert-based signature to bare-key). + /// + /// PoC: 5 bytes each with MSB set make `varint::get32` consume all 5 and + /// return `WSError::ParseError`. Before the fix, the `if let Ok(...)` + /// pattern swallowed this and produced `Ok { certificate_chain: None }`. + #[test] + fn test_malformed_cert_count_is_rejected() { + let mut buf = Vec::new(); + varint::put(&mut buf, 0u64).unwrap(); // key_id = empty + buf.push(ED25519_PK_ID); // alg_id + varint::put_slice(&mut buf, &[1, 2, 3, 4]).unwrap(); // signature + buf.extend_from_slice(&[0x80, 0x80, 0x80, 0x80, 0x80]); // malformed cert_count + + let result = SignatureForHashes::deserialize(&buf); + assert!( + result.is_err(), + "malformed cert_count must error, not silently drop chain — got {:?}", + result + ); + } }