diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index f52f5ba68..9ed28105f 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -2,6 +2,10 @@ ## Unreleased - 2021-xx-xx +### Fixed +* Remove slice creation pointing to potential uninitialized data on h1 encoder. [#2364] + +[#2364]: https://github.com/actix/actix-web/pull/2364 ## 3.0.0-beta.8 - 2021-08-09 ### Fixed diff --git a/actix-http/benches/write-camel-case.rs b/actix-http/benches/write-camel-case.rs index fa4930eb9..ccf09b37e 100644 --- a/actix-http/benches/write-camel-case.rs +++ b/actix-http/benches/write-camel-case.rs @@ -18,7 +18,8 @@ fn bench_write_camel_case(c: &mut Criterion) { group.bench_with_input(BenchmarkId::new("New", i), bts, |b, bts| { b.iter(|| { let mut buf = black_box([0; 24]); - _new::write_camel_case(black_box(bts), &mut buf) + let len = black_box(bts.len()); + _new::write_camel_case(black_box(bts), buf.as_mut_ptr(), len) }); }); } @@ -30,9 +31,12 @@ criterion_group!(benches, bench_write_camel_case); criterion_main!(benches); mod _new { - pub fn write_camel_case(value: &[u8], buffer: &mut [u8]) { + pub fn write_camel_case(value: &[u8], buf: *mut u8, len: usize) { // first copy entire (potentially wrong) slice to output - buffer[..value.len()].copy_from_slice(value); + let buffer = unsafe { + std::ptr::copy_nonoverlapping(value.as_ptr(), buf, len); + std::slice::from_raw_parts_mut(buf, len) + }; let mut iter = value.iter(); diff --git a/actix-http/src/h1/encoder.rs b/actix-http/src/h1/encoder.rs index 4e5c9d238..5e1d47785 100644 --- a/actix-http/src/h1/encoder.rs +++ b/actix-http/src/h1/encoder.rs @@ -175,7 +175,7 @@ pub(crate) trait MessageType: Sized { unsafe { if camel_case { // use Camel-Case headers - write_camel_case(k, from_raw_parts_mut(buf, k_len)); + write_camel_case(k, buf, k_len); } else { write_data(k, buf, k_len); } @@ -473,15 +473,22 @@ impl TransferEncoding { } /// # Safety -/// Callers must ensure that the given length matches given value length. +/// Callers must ensure that the given `len` matches the given `value` length and that `buf` is +/// valid for writes of at least `len` bytes. unsafe fn write_data(value: &[u8], buf: *mut u8, len: usize) { debug_assert_eq!(value.len(), len); copy_nonoverlapping(value.as_ptr(), buf, len); } -fn write_camel_case(value: &[u8], buffer: &mut [u8]) { +/// # Safety +/// Callers must ensure that the given `len` matches the given `value` length and that `buf` is +/// valid for writes of at least `len` bytes. +unsafe fn write_camel_case(value: &[u8], buf: *mut u8, len: usize) { // first copy entire (potentially wrong) slice to output - buffer[..value.len()].copy_from_slice(value); + write_data(value, buf, len); + + // SAFETY: We just initialized the buffer with `value` + let buffer = from_raw_parts_mut(buf, len); let mut iter = value.iter();