Skip to content

Commit 65d2936

Browse files
committed
SPDM: Avoid potential arithmetic overflow
Signed-off-by: Wei Liu <[email protected]>
1 parent b8c6a07 commit 65d2936

File tree

3 files changed

+74
-15
lines changed

3 files changed

+74
-15
lines changed

src/migtd/src/spdm/spdm_req.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,14 @@ async fn send_and_receive_pub_key(spdm_requester: &mut RequesterContext) -> Spdm
249249
.encode(&mut writer)
250250
.map_err(|_| SPDM_STATUS_BUFFER_FULL)?;
251251

252+
let my_pub_key_len = my_pub_key.len();
253+
let my_pub_key_len_u16 = match u16::try_from(my_pub_key_len) {
254+
Ok(v) => v,
255+
Err(_) => return Err(SPDM_STATUS_BUFFER_FULL),
256+
};
252257
let pub_key_element = VdmMessageElement {
253258
element_type: VdmMessageElementType::PubKeyMy,
254-
length: my_pub_key.len() as u16,
259+
length: my_pub_key_len_u16,
255260
};
256261
cnt += pub_key_element
257262
.encode(&mut writer)
@@ -347,18 +352,34 @@ async fn send_and_receive_pub_key(spdm_requester: &mut RequesterContext) -> Spdm
347352
.ok_or(SPDM_STATUS_INVALID_MSG_SIZE)?;
348353

349354
// Provision the public keys to spdm context
355+
let my_pub_key_len = my_pub_key.len();
356+
let my_pub_key_len_u32 = match u32::try_from(my_pub_key_len) {
357+
Ok(v) => v,
358+
Err(_) => return Err(SPDM_STATUS_BUFFER_FULL),
359+
};
360+
if my_pub_key_len > config::MAX_SPDM_CERT_CHAIN_DATA_SIZE {
361+
return Err(SPDM_STATUS_BUFFER_FULL);
362+
}
350363
let mut my_pub_key_prov = SpdmCertChainData {
351-
data_size: my_pub_key.len() as u32,
364+
data_size: my_pub_key_len_u32,
352365
data: [0u8; config::MAX_SPDM_CERT_CHAIN_DATA_SIZE],
353366
};
354-
my_pub_key_prov.data[..my_pub_key.len()].copy_from_slice(&my_pub_key);
367+
my_pub_key_prov.data[..my_pub_key_len].copy_from_slice(&my_pub_key);
355368
spdm_requester.common.provision_info.my_pub_key = Some(my_pub_key_prov);
356369

370+
let peer_pub_key_len = peer_pub_key.len();
371+
let peer_pub_key_len_u32 = match u32::try_from(peer_pub_key_len) {
372+
Ok(v) => v,
373+
Err(_) => return Err(SPDM_STATUS_INVALID_MSG_SIZE),
374+
};
375+
if peer_pub_key_len > config::MAX_SPDM_CERT_CHAIN_DATA_SIZE {
376+
return Err(SPDM_STATUS_INVALID_MSG_SIZE);
377+
}
357378
let mut peer_pub_key_prov = SpdmCertChainData {
358-
data_size: peer_pub_key.len() as u32,
379+
data_size: peer_pub_key_len_u32,
359380
data: [0u8; config::MAX_SPDM_CERT_CHAIN_DATA_SIZE],
360381
};
361-
peer_pub_key_prov.data[..peer_pub_key.len()].copy_from_slice(peer_pub_key);
382+
peer_pub_key_prov.data[..peer_pub_key_len].copy_from_slice(peer_pub_key);
362383
spdm_requester.common.provision_info.peer_pub_key = Some(peer_pub_key_prov);
363384

364385
let vdm_pub_key_src_hash =

src/migtd/src/spdm/spdm_rsp.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,26 +537,42 @@ pub fn handle_exchange_mig_attest_info_req(
537537
.map_err(|_| SPDM_STATUS_BUFFER_FULL)?;
538538

539539
//quote dst
540+
let quote_len = quote_dst.len();
541+
let quote_len_u16 = match u16::try_from(quote_len) {
542+
Ok(v) => v,
543+
Err(_) => return Err(SPDM_STATUS_BUFFER_FULL),
544+
};
540545
let quote_element = VdmMessageElement {
541546
element_type: VdmMessageElementType::QuoteMy,
542-
length: quote_dst.len() as u16,
547+
length: quote_len_u16,
543548
};
544549
cnt += quote_element
545550
.encode(&mut writer)
546551
.map_err(|_| SPDM_STATUS_BUFFER_FULL)?;
552+
if writer.used().checked_add(quote_len).is_none() {
553+
return Err(SPDM_STATUS_BUFFER_FULL);
554+
}
547555
cnt += writer
548556
.extend_from_slice(quote_dst.as_slice())
549557
.ok_or(SPDM_STATUS_BUFFER_FULL)?;
550558

551559
//event log dst
552560
let event_log_dst = get_event_log().ok_or(SPDM_STATUS_INVALID_STATE_LOCAL)?;
561+
let event_log_len = event_log_dst.len();
562+
let event_log_len_u16 = match u16::try_from(event_log_len) {
563+
Ok(v) => v,
564+
Err(_) => return Err(SPDM_STATUS_BUFFER_FULL),
565+
};
553566
let event_log_element = VdmMessageElement {
554567
element_type: VdmMessageElementType::EventLogMy,
555-
length: event_log_dst.len() as u16,
568+
length: event_log_len_u16,
556569
};
557570
cnt += event_log_element
558571
.encode(&mut writer)
559572
.map_err(|_| SPDM_STATUS_BUFFER_FULL)?;
573+
if writer.used().checked_add(event_log_len).is_none() {
574+
return Err(SPDM_STATUS_BUFFER_FULL);
575+
}
560576
cnt += writer
561577
.extend_from_slice(event_log_dst)
562578
.ok_or(SPDM_STATUS_BUFFER_FULL)?;
@@ -565,13 +581,21 @@ pub fn handle_exchange_mig_attest_info_req(
565581
let mig_policy_dst = get_policy().ok_or(SPDM_STATUS_INVALID_STATE_LOCAL)?;
566582
let mig_policy_dst_hash =
567583
digest_sha384(mig_policy_dst).map_err(|_| SPDM_STATUS_CRYPTO_ERROR)?;
584+
let mig_policy_len = mig_policy_dst_hash.len();
585+
let mig_policy_len_u16 = match u16::try_from(mig_policy_len) {
586+
Ok(v) => v,
587+
Err(_) => return Err(SPDM_STATUS_BUFFER_FULL),
588+
};
568589
let mig_policy_element = VdmMessageElement {
569590
element_type: VdmMessageElementType::MigPolicyMy,
570-
length: mig_policy_dst_hash.len() as u16,
591+
length: mig_policy_len_u16,
571592
};
572593
cnt += mig_policy_element
573594
.encode(&mut writer)
574595
.map_err(|_| SPDM_STATUS_BUFFER_FULL)?;
596+
if writer.used().checked_add(mig_policy_len).is_none() {
597+
return Err(SPDM_STATUS_BUFFER_FULL);
598+
}
575599
cnt += writer
576600
.extend_from_slice(&mig_policy_dst_hash)
577601
.ok_or(SPDM_STATUS_BUFFER_FULL)?;

src/migtd/src/spdm/vmcall_msg.rs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,22 +128,31 @@ impl SpdmTransportEncap for VmCallTransportEncap {
128128
VmCallMessageType::SpdmMessage
129129
};
130130

131+
// Check that spdm_buffer.len() fits in u32 to avoid truncation.
132+
let length_u32 = match u32::try_from(spdm_buffer.len()) {
133+
Ok(v) => v,
134+
Err(_) => return Err(SPDM_STATUS_ENCAP_FAIL),
135+
};
131136
let vmcall_msg_header = VmCallMessageHeader {
132137
version: VMCALL_SPDM_VERSION,
133138
msg_type,
134-
length: spdm_buffer.len() as u32,
139+
length: length_u32,
135140
};
136141
vmcall_msg_header
137142
.encode(&mut writer)
138143
.map_err(|_| SPDM_STATUS_ENCAP_FAIL)?;
139144
let header_size = writer.used();
140145

141-
if transport_buffer.len() < header_size + spdm_buffer.len() {
146+
// Prevent arithmetic overflow when adding header_size + payload size.
147+
let total_size = match header_size.checked_add(spdm_buffer.len()) {
148+
Some(v) => v,
149+
None => return Err(SPDM_STATUS_ENCAP_FAIL),
150+
};
151+
if transport_buffer.len() < total_size {
142152
return Err(SPDM_STATUS_ENCAP_FAIL);
143153
}
144-
transport_buffer[header_size..(header_size + spdm_buffer.len())]
145-
.copy_from_slice(&spdm_buffer);
146-
Ok(header_size + spdm_buffer.len())
154+
transport_buffer[header_size..total_size].copy_from_slice(&spdm_buffer);
155+
Ok(total_size)
147156
}
148157

149158
async fn decap(
@@ -156,15 +165,20 @@ impl SpdmTransportEncap for VmCallTransportEncap {
156165
VmCallMessageHeader::read(&mut reader).ok_or(SPDM_STATUS_DECAP_FAIL)?;
157166
let header_size = reader.used();
158167
let payload_size = vmcall_msg_header.length as usize;
159-
if transport_buffer.len() < header_size + payload_size {
168+
// Prevent overflow when computing header_size + payload_size.
169+
let total_needed = match header_size.checked_add(payload_size) {
170+
Some(v) => v,
171+
None => return Err(SPDM_STATUS_DECAP_FAIL),
172+
};
173+
if transport_buffer.len() < total_needed {
160174
return Err(SPDM_STATUS_DECAP_FAIL);
161175
}
162176
let mut spdm_buffer = spdm_buffer.lock();
163177
let spdm_buffer = spdm_buffer.deref_mut();
164178
if spdm_buffer.len() < payload_size {
165179
return Err(SPDM_STATUS_DECAP_FAIL);
166180
}
167-
let payload = &transport_buffer[header_size..(header_size + payload_size)];
181+
let payload = &transport_buffer[header_size..total_needed];
168182
spdm_buffer[..payload_size].copy_from_slice(payload);
169183
let secured_message = vmcall_msg_header.msg_type == VmCallMessageType::SecuredSpdmMessage;
170184

0 commit comments

Comments
 (0)