Skip to content

Commit 513b08e

Browse files
authored
Validate guest-writable fields in try_pop_buffer_into before allocation (#1262)
The back-pointer and flatbuffer size prefix in the shared output buffer are written by the guest and were used without validation, allowing a malicious guest to trigger a ~4 GB host-side allocation. Add bounds checks on both fields before any heap allocation occurs and return descriptive errors on violation. Add unit and integration tests exercising corrupt size prefixes and back-pointers. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent afd325e commit 513b08e

3 files changed

Lines changed: 290 additions & 6 deletions

File tree

src/hyperlight_host/src/mem/shared_mem.rs

Lines changed: 220 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,18 +1121,47 @@ impl HostSharedMemory {
11211121
let last_element_offset_rel: usize =
11221122
self.read::<u64>(last_element_offset_abs - 8)? as usize;
11231123

1124+
// Validate element offset (guest-writable): must be in [8, stack_pointer_rel - 16]
1125+
// to leave room for the 8-byte back-pointer plus at least 8 bytes of element data
1126+
// (the minimum for a size-prefixed flatbuffer: 4-byte prefix + 4-byte root offset).
1127+
if last_element_offset_rel > stack_pointer_rel.saturating_sub(16)
1128+
|| last_element_offset_rel < 8
1129+
{
1130+
return Err(new_error!(
1131+
"Corrupt buffer back-pointer: element offset {} is outside valid range [8, {}].",
1132+
last_element_offset_rel,
1133+
stack_pointer_rel.saturating_sub(16),
1134+
));
1135+
}
1136+
11241137
// make it absolute
11251138
let last_element_offset_abs = last_element_offset_rel + buffer_start_offset;
11261139

1140+
// Max bytes the element can span (excluding the 8-byte back-pointer).
1141+
let max_element_size = stack_pointer_rel - last_element_offset_rel - 8;
1142+
11271143
// Get the size of the flatbuffer buffer from memory
11281144
let fb_buffer_size = {
1129-
let size_i32 = self.read::<u32>(last_element_offset_abs)? + 4;
1130-
// ^^^ flatbuffer byte arrays are prefixed by 4 bytes
1131-
// indicating its size, so, to get the actual size, we need
1132-
// to add 4.
1133-
usize::try_from(size_i32)
1145+
let raw_prefix = self.read::<u32>(last_element_offset_abs)?;
1146+
// flatbuffer byte arrays are prefixed by 4 bytes indicating
1147+
// the remaining size; add 4 for the prefix itself.
1148+
let total = raw_prefix.checked_add(4).ok_or_else(|| {
1149+
new_error!(
1150+
"Corrupt buffer size prefix: value {} overflows when adding 4-byte header.",
1151+
raw_prefix
1152+
)
1153+
})?;
1154+
usize::try_from(total)
11341155
}?;
11351156

1157+
if fb_buffer_size > max_element_size {
1158+
return Err(new_error!(
1159+
"Corrupt buffer size prefix: flatbuffer claims {} bytes but the element slot is only {} bytes.",
1160+
fb_buffer_size,
1161+
max_element_size
1162+
));
1163+
}
1164+
11361165
let mut result_buffer = vec![0; fb_buffer_size];
11371166

11381167
self.copy_to_slice(&mut result_buffer, last_element_offset_abs)?;
@@ -1631,6 +1660,192 @@ mod tests {
16311660
}
16321661
}
16331662

1663+
/// Bounds checking for `try_pop_buffer_into` against corrupt guest data.
1664+
mod try_pop_buffer_bounds {
1665+
use super::*;
1666+
1667+
#[derive(Debug, PartialEq)]
1668+
struct RawBytes(Vec<u8>);
1669+
1670+
impl TryFrom<&[u8]> for RawBytes {
1671+
type Error = String;
1672+
fn try_from(value: &[u8]) -> std::result::Result<Self, Self::Error> {
1673+
Ok(RawBytes(value.to_vec()))
1674+
}
1675+
}
1676+
1677+
/// Create a buffer with stack pointer initialized to 8 (empty).
1678+
fn make_buffer(mem_size: usize) -> super::super::HostSharedMemory {
1679+
let eshm = ExclusiveSharedMemory::new(mem_size).unwrap();
1680+
let (hshm, _) = eshm.build();
1681+
hshm.write::<u64>(0, 8u64).unwrap();
1682+
hshm
1683+
}
1684+
1685+
#[test]
1686+
fn normal_push_pop_roundtrip() {
1687+
let mem_size = 4096;
1688+
let mut hshm = make_buffer(mem_size);
1689+
1690+
// Size-prefixed flatbuffer-like payload: [size: u32 LE][payload]
1691+
let payload = b"hello";
1692+
let mut data = Vec::new();
1693+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1694+
data.extend_from_slice(payload);
1695+
1696+
hshm.push_buffer(0, mem_size, &data).unwrap();
1697+
let result: RawBytes = hshm.try_pop_buffer_into(0, mem_size).unwrap();
1698+
assert_eq!(result.0, data);
1699+
}
1700+
1701+
#[test]
1702+
fn malicious_flatbuffer_size_prefix() {
1703+
let mem_size = 4096;
1704+
let mut hshm = make_buffer(mem_size);
1705+
1706+
let payload = b"small";
1707+
let mut data = Vec::new();
1708+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1709+
data.extend_from_slice(payload);
1710+
hshm.push_buffer(0, mem_size, &data).unwrap();
1711+
1712+
// Corrupt size prefix at element start (offset 8) to near u32::MAX.
1713+
hshm.write::<u32>(8, 0xFFFF_FFFBu32).unwrap(); // +4 = 0xFFFF_FFFF
1714+
1715+
let result: Result<RawBytes> = hshm.try_pop_buffer_into(0, mem_size);
1716+
let err_msg = format!("{}", result.unwrap_err());
1717+
assert!(
1718+
err_msg.contains("Corrupt buffer size prefix: flatbuffer claims 4294967295 bytes but the element slot is only 9 bytes"),
1719+
"Unexpected error message: {}",
1720+
err_msg
1721+
);
1722+
}
1723+
1724+
#[test]
1725+
fn malicious_element_offset_too_small() {
1726+
let mem_size = 4096;
1727+
let mut hshm = make_buffer(mem_size);
1728+
1729+
let payload = b"test";
1730+
let mut data = Vec::new();
1731+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1732+
data.extend_from_slice(payload);
1733+
hshm.push_buffer(0, mem_size, &data).unwrap();
1734+
1735+
// Corrupt back-pointer (offset 16) to 0 (before valid range).
1736+
hshm.write::<u64>(16, 0u64).unwrap();
1737+
1738+
let result: Result<RawBytes> = hshm.try_pop_buffer_into(0, mem_size);
1739+
let err_msg = format!("{}", result.unwrap_err());
1740+
assert!(
1741+
err_msg.contains(
1742+
"Corrupt buffer back-pointer: element offset 0 is outside valid range [8, 8]"
1743+
),
1744+
"Unexpected error message: {}",
1745+
err_msg
1746+
);
1747+
}
1748+
1749+
#[test]
1750+
fn malicious_element_offset_past_stack_pointer() {
1751+
let mem_size = 4096;
1752+
let mut hshm = make_buffer(mem_size);
1753+
1754+
let payload = b"test";
1755+
let mut data = Vec::new();
1756+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1757+
data.extend_from_slice(payload);
1758+
hshm.push_buffer(0, mem_size, &data).unwrap();
1759+
1760+
// Corrupt back-pointer (offset 16) to 9999 (past stack pointer 24).
1761+
hshm.write::<u64>(16, 9999u64).unwrap();
1762+
1763+
let result: Result<RawBytes> = hshm.try_pop_buffer_into(0, mem_size);
1764+
let err_msg = format!("{}", result.unwrap_err());
1765+
assert!(
1766+
err_msg.contains(
1767+
"Corrupt buffer back-pointer: element offset 9999 is outside valid range [8, 8]"
1768+
),
1769+
"Unexpected error message: {}",
1770+
err_msg
1771+
);
1772+
}
1773+
1774+
#[test]
1775+
fn malicious_flatbuffer_size_off_by_one() {
1776+
let mem_size = 4096;
1777+
let mut hshm = make_buffer(mem_size);
1778+
1779+
let payload = b"abcd";
1780+
let mut data = Vec::new();
1781+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1782+
data.extend_from_slice(payload);
1783+
hshm.push_buffer(0, mem_size, &data).unwrap();
1784+
1785+
// Corrupt size prefix: claim 5 bytes (total 9), exceeding the 8-byte slot.
1786+
hshm.write::<u32>(8, 5u32).unwrap(); // fb_buffer_size = 5 + 4 = 9
1787+
1788+
let result: Result<RawBytes> = hshm.try_pop_buffer_into(0, mem_size);
1789+
let err_msg = format!("{}", result.unwrap_err());
1790+
assert!(
1791+
err_msg.contains("Corrupt buffer size prefix: flatbuffer claims 9 bytes but the element slot is only 8 bytes"),
1792+
"Unexpected error message: {}",
1793+
err_msg
1794+
);
1795+
}
1796+
1797+
/// Back-pointer just below stack_pointer causes underflow in
1798+
/// `stack_pointer_rel - last_element_offset_rel - 8`.
1799+
#[test]
1800+
fn back_pointer_near_stack_pointer_underflow() {
1801+
let mem_size = 4096;
1802+
let mut hshm = make_buffer(mem_size);
1803+
1804+
let payload = b"test";
1805+
let mut data = Vec::new();
1806+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1807+
data.extend_from_slice(payload);
1808+
hshm.push_buffer(0, mem_size, &data).unwrap();
1809+
1810+
// stack_pointer_rel = 24. Set back-pointer to 23 (> 24 - 16 = 8, so rejected).
1811+
hshm.write::<u64>(16, 23u64).unwrap();
1812+
1813+
let result: Result<RawBytes> = hshm.try_pop_buffer_into(0, mem_size);
1814+
let err_msg = format!("{}", result.unwrap_err());
1815+
assert!(
1816+
err_msg.contains(
1817+
"Corrupt buffer back-pointer: element offset 23 is outside valid range [8, 8]"
1818+
),
1819+
"Unexpected error message: {}",
1820+
err_msg
1821+
);
1822+
}
1823+
1824+
/// Size prefix of 0xFFFF_FFFD causes u32 overflow: 0xFFFF_FFFD + 4 wraps.
1825+
#[test]
1826+
fn size_prefix_u32_overflow() {
1827+
let mem_size = 4096;
1828+
let mut hshm = make_buffer(mem_size);
1829+
1830+
let payload = b"test";
1831+
let mut data = Vec::new();
1832+
data.extend_from_slice(&(payload.len() as u32).to_le_bytes());
1833+
data.extend_from_slice(payload);
1834+
hshm.push_buffer(0, mem_size, &data).unwrap();
1835+
1836+
// Write 0xFFFF_FFFD as size prefix: checked_add(4) returns None.
1837+
hshm.write::<u32>(8, 0xFFFF_FFFDu32).unwrap();
1838+
1839+
let result: Result<RawBytes> = hshm.try_pop_buffer_into(0, mem_size);
1840+
let err_msg = format!("{}", result.unwrap_err());
1841+
assert!(
1842+
err_msg.contains("Corrupt buffer size prefix: value 4294967293 overflows when adding 4-byte header"),
1843+
"Unexpected error message: {}",
1844+
err_msg
1845+
);
1846+
}
1847+
}
1848+
16341849
#[cfg(target_os = "linux")]
16351850
mod guard_page_crash_test {
16361851
use crate::mem::shared_mem::{ExclusiveSharedMemory, SharedMemory};

src/hyperlight_host/tests/integration_test.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,42 @@ fn guest_outb_with_invalid_port_poisons_sandbox() {
578578
});
579579
}
580580

581+
#[test]
582+
fn corrupt_output_size_prefix_rejected() {
583+
with_rust_sandbox(|mut sbox| {
584+
let res = sbox.call::<i32>("CorruptOutputSizePrefix", ());
585+
assert!(
586+
res.is_err(),
587+
"Expected error when guest corrupts size prefix, got: {:?}",
588+
res,
589+
);
590+
let err_msg = format!("{:?}", res.unwrap_err());
591+
assert!(
592+
err_msg.contains("Corrupt buffer size prefix: flatbuffer claims 4294967295 bytes but the element slot is only 8 bytes"),
593+
"Unexpected error message: {err_msg}"
594+
);
595+
});
596+
}
597+
598+
#[test]
599+
fn corrupt_output_back_pointer_rejected() {
600+
with_rust_sandbox(|mut sbox| {
601+
let res = sbox.call::<i32>("CorruptOutputBackPointer", ());
602+
assert!(
603+
res.is_err(),
604+
"Expected error when guest corrupts back-pointer, got: {:?}",
605+
res,
606+
);
607+
let err_msg = format!("{:?}", res.unwrap_err());
608+
assert!(
609+
err_msg.contains(
610+
"Corrupt buffer back-pointer: element offset 57005 is outside valid range [8, 8]"
611+
),
612+
"Unexpected error message: {err_msg}"
613+
);
614+
});
615+
}
616+
581617
#[test]
582618
fn guest_panic_no_alloc() {
583619
let heap_size = 0x4000;

src/tests/rust_guests/simpleguest/src/main.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ use hyperlight_guest_bin::host_comm::{
5252
print_output_with_host_print, read_n_bytes_from_user_memory,
5353
};
5454
use hyperlight_guest_bin::memory::malloc;
55-
use hyperlight_guest_bin::{guest_function, guest_logger, host_function};
55+
use hyperlight_guest_bin::{GUEST_HANDLE, guest_function, guest_logger, host_function};
5656
use log::{LevelFilter, error};
5757
use tracing::{Span, instrument};
5858

@@ -804,6 +804,39 @@ fn fuzz_guest_trace(max_depth: u32, msg: String) -> u32 {
804804
fuzz_traced_function(0, max_depth, &msg)
805805
}
806806

807+
#[guest_function("CorruptOutputSizePrefix")]
808+
fn corrupt_output_size_prefix() -> i32 {
809+
unsafe {
810+
let peb_ptr = core::ptr::addr_of!(GUEST_HANDLE).read().peb().unwrap();
811+
let output_stack_ptr = (*peb_ptr).output_stack.ptr as *mut u8;
812+
813+
// Write a fake stack entry with a ~4 GB size prefix (0xFFFF_FFFB + 4).
814+
let buf = core::slice::from_raw_parts_mut(output_stack_ptr, 24);
815+
buf[0..8].copy_from_slice(&24_u64.to_le_bytes());
816+
buf[8..12].copy_from_slice(&0xFFFF_FFFBu32.to_le_bytes());
817+
buf[12..16].copy_from_slice(&[0u8; 4]);
818+
buf[16..24].copy_from_slice(&8_u64.to_le_bytes());
819+
820+
core::arch::asm!("hlt", options(noreturn));
821+
}
822+
}
823+
824+
#[guest_function("CorruptOutputBackPointer")]
825+
fn corrupt_output_back_pointer() -> i32 {
826+
unsafe {
827+
let peb_ptr = core::ptr::addr_of!(GUEST_HANDLE).read().peb().unwrap();
828+
let output_stack_ptr = (*peb_ptr).output_stack.ptr as *mut u8;
829+
830+
// Write a fake stack entry with back-pointer 0xDEAD (past stack pointer 24).
831+
let buf = core::slice::from_raw_parts_mut(output_stack_ptr, 24);
832+
buf[0..8].copy_from_slice(&24_u64.to_le_bytes());
833+
buf[8..16].copy_from_slice(&[0u8; 8]);
834+
buf[16..24].copy_from_slice(&0xDEAD_u64.to_le_bytes());
835+
836+
core::arch::asm!("hlt", options(noreturn));
837+
}
838+
}
839+
807840
// Interprets the given guest function call as a host function call and dispatches it to the host.
808841
fn fuzz_host_function(func: FunctionCall) -> Result<Vec<u8>> {
809842
let mut params = func.parameters.unwrap();

0 commit comments

Comments
 (0)