Skip to content

fix(migtd): input validation and robustness from security audit#942

Draft
MichalTarnacki wants to merge 1 commit into
intel:mainfrom
MichalTarnacki:fix/impl-bugs-migtd
Draft

fix(migtd): input validation and robustness from security audit#942
MichalTarnacki wants to merge 1 commit into
intel:mainfrom
MichalTarnacki:fix/impl-bugs-migtd

Conversation

@MichalTarnacki

@MichalTarnacki MichalTarnacki commented Jul 7, 2026

Copy link
Copy Markdown
Contributor
# Assessed Severity Bug Call Chain / Location Fix Classification Disposition Reason
1 High Unbounded vec![0;peer_u32] alloc in pre-TLS receive_pre_session_data_packet handle_pre_mig -> exchange_msk -> pre_session_data_exchange -> receive_pre_session_data_packet Clamp header.length to MAX_PRE_SESSION_PAYLOAD (e.g. 64KiB) before alloc; use try_reserve. weakness The unbounded allocation from a peer-controlled u32 length field causes OOM panic, but the impact is purely denial of service. This pre-TLS exchange runs over virtio-vsock transport fully controlled by the VMM, which already possesses unlimited DoS capability through scheduling denial and VMCALL non-response. The fix is good input validation at a trust boundary, but does not prevent an attack class the adversary lacked before.
2 Medium parse_events indexes peer event_data[0]/[1..1+desc_size]/[..4] without length check exchange_msk -> ratls::client -> verify_peer_cert -> mig_policy::authenticate_remote -> parse_events Use event_data.get(0)/.get(1..1+desc_size)/.get(..4) and return None on short data. weakness The unchecked event_data[0] in parse_events() is a real crash path on malformed event data, but it is not practically exploitable in production. The peer's event log is embedded in their RA-TLS certificate and verified against RTMRs from their TDX quote before parse_events() runs. A peer capable of crafting a malicious event log that passes RTMR replay must be running modified firmware or a modified MigTD binary, both of which change MRTD — causing policy rejection before the vulnerable code is reached.
3 High VMM-set log_max_level + shared logarea header TOCTOU -> entrylog slice-index panic kills all sessions handle_pre_mig -> log::set_max_level -> <any log!()> -> VmmLoggerBackend::log -> entrylog Copy LogAreaBufferHeader to private stack before use and clamp start_offset/end_offset to [LOGAREABUFFERHEADERSIZE,PAGE_SIZE); reject log_max_level changes after init. weakness The log buffer is shared memory but the impact is DoS only — Rust's slice bounds checking prevents any out-of-bounds write, so VMM-crafted offsets can only trigger a panic. Since the TDX threat model does not guarantee availability and the VMM already has unlimited DoS capability over any TD (scheduling control, interrupt withholding, VMCALL non-response), the panic does not grant the VMM any new security-relevant capability. The volatile read plus clamp is defense-in-depth that converts a crash into graceful recover
4 Low Ok(0) infinite busy-loop in receive_pre_session_data defeats with_timeout handle_pre_mig -> pre_session_data_exchange -> receive_hello_packet -> receive_pre_session_data -> TransportType::read Treat n==0 as EOF: if n==0 { return Err(NetworkError) } after each read/write in the while loop. weakness The infinite busy-loop on EOF is a correctness bug that defeats the async timeout mechanism, but its impact is purely availability. The VMM controls the vsock transport layer and can simulate connection close at will, yet it already possesses unlimited DoS capability over MigTD through scheduling denial and VMCALL non-response. The fix converts an unrecoverable hang into a clean error return, which is good defensive coding, but does not block an attack class the adversary lacked before.
5 Low Divide-by-zero on VMM-controlled CPUID.15H.EAX in schedule_timeout init_sys_tick -> schedule_timeout -> __cpuid_count(0x15) Validate cpuid.eax!=0 (and ebx/ecx sane) before division; fall back to a fixed TSC freq or return Err. weakness The finding is valid in that CPUID.15H is virtualized by the VMM in TDX and a malicious VMM could return eax=0, triggering a division-by-zero CPU exception in cpuid.ebx / cpuid.eax. However, the security impact is negligible: a VMM that wants to crash or DoS MigTD has countless other mechanisms (ignore VCALLs, corrupt shared memory, withhold interrupts, return garbage from any other virtualized interface). This check closes one specific crash vector but does not materially change the security posture. The fix is correct and cheap, but it is defensive hardening rather than a meaningful vulnerability remediation.
6 Medium .unwrap() on pci::find_device -- VMM omits device -> boot panic main -> init_devices -> virtio_vsock_device_init -> pci::find_device Replace .unwrap()/.expect() with ? propagating to a graceful init-failure report (panic_with_guest_crash_reg_report or Err). weakness he finding is valid in that pci::find_device can return None if VMM omits the expected device, and the original .unwrap() panicked without reporting the failure reason through guest crash registers. However, the security impact is negligible: a VMM that omits the required virtio device is effectively denying migration to itself, and the TD cannot continue without it — halting is the only correct outcome regardless.
7 Low get_ccel .expect() panics if td-shim ACPI handoff lacks CCEL or has <4-byte entry do_measurements -> get_event_log_mut -> get_ccel -> get_acpi_tables Use t.get(..4)==Some(b"CCEL") and return None instead of .expect(). weakness The event_log_slice function previously called from_raw_parts_mut(ccel.lasa, ccel.laml) without validating the CCEL fields, which is technically undefined behavior if lasa is null or laml is unreasonably large. However, the CCEL ACPI table is created by td-shim (firmware), which is part of the TCB and measured into MRTD at boot. The VMM does not control lasa/laml — they are set by firmware when it allocates the event log area in TD private memory. The only scenario where these values would be invalid is a firmware bug, which already compromises the entire security model.
8 Low VmcallServiceResponse::try_read validates length but never truncates stored slic -> try_read Slice input to &data[..length] before storing in VmcallServiceResponse. weakness The response buffer resides in shared memory fully controlled by the VMM. The VMM writes both the length field and all data bytes in the buffer. Without the truncation, the VMM could make MigTD parse data beyond the declared length — but since the VMM also placed that data there, it gains no additional capability it did not already have by simply declaring a larger length. The fix enforces GHCI protocol framing discipline but does not block any attack that the VMM could not already perform through the legitimate protocol interface.
9 High event_log_slice builds &'static mut [u8] from ACPI CCEL lasa/laml unvalidated -> event_log_slice Validate lasa/laml against td-shim memory map; cache slice in OnceCell to avoid aliasing; fix [..size+1] to [..size]. weakness The lasa and laml values come from the CCEL ACPI table created by td-shim, which is measured firmware in the TCB — the VMM cannot control or modify them because they reside in TD private memory. A null lasa would require a td-shim bug, which would change the MRTD measurement and be rejected by policy. The check prevents undefined behavior (null slice construction) only in the hypothetical case of a faulty firmware, making it pure defense-in-depth with no realistic attack path.
10 Low get_ccel .expect() panics if ACPI CCEL absent or table <4 bytes -> get_ccel Return Option/Result; caller degrades gracefully (skip event-log feature) instead of panic. weakness The CCEL ACPI table is created by td-shim firmware, which is part of the TCB and measured into MRTD at boot. Its absence or a sub-4-byte ACPI table entry would indicate a firmware bug, not a VMM attack — the VMM cannot inject or remove ACPI tables from TD private memory after launch. The fix improves error handling quality (graceful None propagation instead of a bare panic without crash-register reporting), but does not close an attacker-reachable vulnerability. On any correctly-configured production system the .expect() would never fire; the improvement is purely in debuggability and robustness against hypothetical firmware defects.

- clamp pre-session payload length before alloc
- parse_events safe indexing on peer event data
- log_max_level/logarea header TOCTOU clamp
- receive_pre_session_data treat n==0 as EOF
- schedule_timeout validate CPUID.15H.EAX!=0
- pci::find_device propagate init failure gracefully
- get_ccel/event_log_slice validate CCEL fields
- VmcallServiceResponse::try_read truncate to length

Signed-off-by: Michal Tarnacki <[email protected]>
Co-authored-by: GitHub Copilot <[email protected]>
@MichalTarnacki MichalTarnacki marked this pull request as draft July 9, 2026 13:22
}

let pre_session_data_payload_size = header.length as usize;
const MAX_PRE_SESSION_PAYLOAD: usize = 64 * 1024; // 64 KiB

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this limit is too low and it breaks rebinding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant