Skip to content
This repository was archived by the owner on Oct 26, 2021. It is now read-only.

Commit 087a62d

Browse files
committed
fix(sallyport): mark copy_into_slice unsafe
`Cursor::copy_into_slice` has to be unsafe, otherwise it is possible to fill in a slice of e.g. bools with invalid values using safe rust code. Also add a test calling `Cursor::copy_into_slice()`. Signed-off-by: Harald Hoyer <[email protected]>
1 parent 1c83c68 commit 087a62d

File tree

3 files changed

+60
-12
lines changed

3 files changed

+60
-12
lines changed

internal/shim-sgx/src/handler.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,10 @@ impl<'a> SyscallHandler for Handler<'a> {
397397
let mut ti = [0u8; 512];
398398
let ti_len = ti.len();
399399
let c = self.new_cursor();
400-
c.copy_into_slice(buf_len, ti.as_mut(), ti_len)
401-
.or(Err(libc::EFAULT))?;
400+
unsafe {
401+
c.copy_into_slice(buf_len, ti.as_mut(), ti_len)
402+
.or(Err(libc::EFAULT))?;
403+
}
402404
// ... Code to generate Report goes here ...
403405

404406
// Request Quote from host
@@ -419,8 +421,10 @@ impl<'a> SyscallHandler for Handler<'a> {
419421
self.attacked()
420422
}
421423

422-
c.copy_into_slice(buf_len, buf.as_mut(), result_len)
423-
.or(Err(libc::EFAULT))?;
424+
unsafe {
425+
c.copy_into_slice(buf_len, buf.as_mut(), result_len)
426+
.or(Err(libc::EFAULT))?;
427+
}
424428

425429
let rep: sallyport::Reply = Ok([result[0], SGX_TECH.into()]).into();
426430
sallyport::Result::from(rep)

internal/syscall/src/lib.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,10 @@ pub trait SyscallHandler: AddressValidator + Sized {
201201
}
202202

203203
let c = self.new_cursor();
204-
c.copy_into_slice(count, buf[..count].as_mut(), result_len)
205-
.or(Err(libc::EFAULT))?;
204+
unsafe {
205+
c.copy_into_slice(count, buf[..count].as_mut(), result_len)
206+
.or(Err(libc::EFAULT))?;
207+
}
206208

207209
Ok(ret)
208210
}
@@ -590,8 +592,10 @@ pub trait SyscallHandler: AddressValidator + Sized {
590592

591593
let c = self.new_cursor();
592594

593-
c.copy_into_slice(nfds as _, fds, nfds as _)
594-
.or(Err(libc::EMSGSIZE))?;
595+
unsafe {
596+
c.copy_into_slice(nfds as _, fds, nfds as _)
597+
.or(Err(libc::EMSGSIZE))?;
598+
}
595599

596600
Ok(result)
597601
}

src/sallyport/mod.rs

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,12 @@ impl<'short, 'a: 'short> Cursor<'a> {
250250
}
251251

252252
/// Copies data from a slice into the cursor buffer using self.alloc().
253+
///
254+
/// # Safety
255+
/// The caller has to ensure the `Cursor` contains valid data of the type
256+
/// of the destination slice.
253257
#[allow(dead_code)]
254-
pub fn copy_into_slice<T: Copy>(
258+
pub unsafe fn copy_into_slice<T: Copy>(
255259
self,
256260
src_len: usize,
257261
dst: &mut [T],
@@ -262,9 +266,7 @@ impl<'short, 'a: 'short> Cursor<'a> {
262266

263267
let (c, src) = self.alloc::<T>(src_len)?;
264268

265-
unsafe {
266-
core::ptr::copy_nonoverlapping(src.as_ptr(), dst.as_mut_ptr() as _, dst_len);
267-
}
269+
core::ptr::copy_nonoverlapping(src.as_ptr(), dst.as_mut_ptr() as _, dst_len);
268270

269271
Ok(c)
270272
}
@@ -566,4 +568,42 @@ mod tests {
566568

567569
Ok(())
568570
}
571+
572+
#[test]
573+
fn copy_into_slice() -> std::result::Result<(), OutOfSpace> {
574+
let mut block = Block::default();
575+
576+
let c = block.cursor();
577+
let (c, slab1) = c
578+
.copy_from_slice::<usize>(&[1, 2])
579+
.expect("allocate slab of 2 usize values for the first time");
580+
581+
let (c, slab2) = c
582+
.copy_from_slice::<usize>(&[3, 4])
583+
.expect("allocate slab of 2 usize values for the second time");
584+
585+
let (_c, slab3) = c
586+
.copy_from_slice::<usize>(&[5, 6])
587+
.expect("allocate slab of 2 usize values for the third time");
588+
589+
assert_eq!(slab1, &[1, 2]);
590+
assert_eq!(slab2, &[3, 4]);
591+
assert_eq!(slab3, &[5, 6]);
592+
593+
let c = block.cursor();
594+
595+
let mut slab_all = [0usize; 3];
596+
597+
let c = unsafe { c.copy_into_slice::<usize>(4, &mut slab_all, 3) }?;
598+
599+
assert_eq!(&slab_all, &[1, 2, 3]);
600+
601+
let mut slab_all = [0usize; 2];
602+
603+
let _ = unsafe { c.copy_into_slice::<usize>(2, &mut slab_all, 2) }?;
604+
605+
assert_eq!(&slab_all, &[5, 6]);
606+
607+
Ok(())
608+
}
569609
}

0 commit comments

Comments
 (0)