-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Summary
A scan of the codebase identified several locations where plain + or - arithmetic is performed on usize values derived from untrusted guest input. In debug builds, integer overflows cause panics; in release builds, they silently wrap, potentially producing incorrect pointer values that could be exploited.
The safe pattern is to use .wrapping_add() / .wrapping_sub() instead of + / - on guest-derived usize values.
Findings
🔴 HIGH — litebox_shim_linux/src/syscalls/process.rs (lines 478, 494)
Unsafe code:
// line 471
let entry_head = head_ptr + offset_of!(litebox_common_linux::RobustListHead, list);
// line 478
crate::ConstPtr::from_usize(entry.as_usize() + futex_offset),
// line 494
crate::ConstPtr::from_usize(pending.as_usize() + futex_offset),futex_offset is head.futex_offset, read directly from the guest-provided RobustListHead structure. entry and pending are guest-controlled pointers from the same structure. A guest can provide entry = usize::MAX and futex_offset = 1, causing a debug-mode panic on overflow.
head_ptr + offset_of!(...) (line 471): head_ptr is a guest-provided address passed through sys_set_robust_list. Although offset_of!(RobustListHead, list) == 0, the addition is still technically unchecked.
Suggested fix:
let entry_head = head_ptr.wrapping_add(offset_of!(litebox_common_linux::RobustListHead, list));
// ...
crate::ConstPtr::from_usize(entry.as_usize().wrapping_add(futex_offset)),
// ...
crate::ConstPtr::from_usize(pending.as_usize().wrapping_add(futex_offset)),🔴 HIGH — litebox_shim_optee/src/msg_handler.rs (line 706)
Unsafe code:
let size: usize = tmem.size.truncate();
// ...
let page_offset = phys_addr_usize % PAGE_SIZE; // bounded to [0, PAGE_SIZE-1]
let num_pages = (page_offset + size).div_ceil(PAGE_SIZE);tmem.size comes from the untrusted normal world (via OpteeMsgParamTmem). If size is close to usize::MAX, adding page_offset overflows. There is no bounds check on size before this arithmetic.
Suggested fix:
let num_pages = page_offset
.wrapping_add(size)
.div_ceil(PAGE_SIZE);Or, better: validate size before use:
let total = page_offset.checked_add(size).ok_or(OpteeSmcReturnCode::EBadAddr)?;
let num_pages = total.div_ceil(PAGE_SIZE);🟡 MEDIUM — litebox_shim_linux/src/syscalls/signal/mod.rs (lines 266, 353)
Unsafe code:
// line 266 (in `is_on_stack`)
let stack_end = stack.sp + stack.size;
// line 353 (in `deliver_signal`)
let sp = if switch_stacks {
altstack.sp + altstack.size
} else { ... };SigAltStack::sp and SigAltStack::size ultimately come from the guest via sys_sigaltstack. The set_sigaltstack function validates the sum with checked_add before storing, so the stored values should not overflow in practice. However, these call sites re-perform the addition without wrapping arithmetic. Any future code path that stores a SigAltStack without going through the validator would silently introduce a panic.
Suggested fix:
// line 266
let stack_end = stack.sp.wrapping_add(stack.size);
// line 353
altstack.sp.wrapping_add(altstack.size)Affected Files
| File | Lines | Severity | Guest-derived values |
|---|---|---|---|
litebox_shim_linux/src/syscalls/process.rs |
471, 478, 494 | 🔴 HIGH | head_ptr (from sys_set_robust_list), entry, pending, futex_offset (from RobustListHead) |
litebox_shim_optee/src/msg_handler.rs |
706 | 🔴 HIGH | tmem.size (from normal world OpteeMsgParamTmem) |
litebox_shim_linux/src/syscalls/signal/mod.rs |
266, 353 | 🟡 MEDIUM | SigAltStack::sp, SigAltStack::size (from sys_sigaltstack, validated before storage) |
References
- Existing safe examples in the codebase:
litebox_shim_linux/src/syscalls/signal/x86.rsandx86_64.rsusewrapping_sub/wrapping_addconsistently for allframe_addrarithmetic. set_sigaltstackalready useschecked_addfor validation — a good pattern to apply at the arithmetic sites too.
Generated by Integer Overflow Scanner