Skip to content

Commit f821ba6

Browse files
[48.0.0] Backports for security fixes (#14175)
* Limit buffered writes in http/files This commit adds limits to the amount of data buffered from a guest on the host when guests write to WASIp3 streams for files and http bodies. This ensures that the guest can't control how much is allocated on the host, for example, but rather it's limited to a fixed amount. Co-authored-by: Till Schneidereit <till@tillschneidereit.net> * Update cap-std dependencies * Fix MSRV --------- Co-authored-by: Till Schneidereit <till@tillschneidereit.net>
1 parent a12fff3 commit f821ba6

11 files changed

Lines changed: 236 additions & 22 deletions

File tree

Cargo.lock

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,8 @@ wasip1 = { version = "1.0.0", default-features = false }
367367
# Note that `cap-fs-ext` should be avoided where possible to use
368368
# `cap-primitives` instead.
369369
target-lexicon = "0.13.5"
370-
cap-primitives = "4.0.2"
371-
cap-fs-ext-avoid-using-this = { version = "4.0.2", package = 'cap-fs-ext' }
370+
cap-primitives = "4.0.3"
371+
cap-fs-ext-avoid-using-this = { version = "4.0.3", package = 'cap-fs-ext' }
372372
rustix = "1.1.4"
373373
# wit-bindgen:
374374
wit-bindgen = { version = "0.60.0", default-features = false }
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
use futures::join;
2+
use test_programs::p3::wasi::filesystem::types::{
3+
Descriptor, DescriptorFlags, OpenFlags, PathFlags,
4+
};
5+
use test_programs::p3::{wasi, wit_stream};
6+
use wit_bindgen::StreamResult;
7+
8+
struct Component;
9+
10+
test_programs::p3::export!(Component);
11+
12+
impl test_programs::p3::exports::wasi::cli::run::Guest for Component {
13+
async fn run() -> Result<(), ()> {
14+
let preopens = wasi::filesystem::preopens::get_directories();
15+
let (dir, _) = &preopens[0];
16+
test_chunked_write(dir, "chunked_write.txt").await;
17+
Ok(())
18+
}
19+
}
20+
21+
fn bytes(offset: &mut usize, len: usize) -> Vec<u8> {
22+
let mut buf = Vec::with_capacity(len);
23+
for i in 0..len {
24+
buf.push(((*offset + i) % 251) as u8);
25+
}
26+
*offset += len;
27+
buf
28+
}
29+
30+
async fn test_chunked_write(dir: &Descriptor, filename: &str) {
31+
let mut len = 16;
32+
let mut pos = 0;
33+
34+
let file = dir
35+
.open_at(
36+
PathFlags::empty(),
37+
filename.to_string(),
38+
OpenFlags::CREATE,
39+
DescriptorFlags::READ | DescriptorFlags::WRITE,
40+
)
41+
.await
42+
.expect("creating a file for writing");
43+
44+
let (mut tx, rx) = wit_stream::new();
45+
join! {
46+
async {
47+
file.write_via_stream(rx, 0).await.unwrap();
48+
},
49+
async {
50+
loop {
51+
// Wasmtime shouldn't buffer this much data by default on the
52+
// host, something should have done a short write earlier.
53+
assert!(len <= 128 << 20);
54+
let (result, remaining) = tx.write(bytes(&mut pos, len)).await;
55+
assert!(matches!(result, StreamResult::Complete(_)), "bad result {result:?}");
56+
if remaining.remaining() == 0 {
57+
len = len.checked_mul(2).unwrap();
58+
} else {
59+
pos -= remaining.remaining();
60+
break;
61+
}
62+
}
63+
drop(tx);
64+
},
65+
};
66+
67+
let expected = bytes(&mut 0, pos);
68+
let (rx, result) = file.read_via_stream(0);
69+
let read_back = rx.collect().await;
70+
result.await.unwrap();
71+
72+
assert_eq!(
73+
read_back.len(),
74+
expected.len(),
75+
"wrong number of bytes read back"
76+
);
77+
assert!(
78+
read_back == expected,
79+
"contents differ after a chunked write"
80+
);
81+
}
82+
83+
fn main() {
84+
unreachable!()
85+
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
use futures::join;
2+
use test_programs::p3::wasi::http::client;
3+
use test_programs::p3::wasi::http::types::{Headers, Method, Request, Response, Scheme};
4+
use test_programs::p3::{wit_future, wit_stream};
5+
use wit_bindgen::StreamResult;
6+
7+
struct Component;
8+
9+
test_programs::p3::export!(Component);
10+
11+
fn bytes(offset: &mut usize, len: usize) -> Vec<u8> {
12+
let mut buf = Vec::with_capacity(len);
13+
for i in 0..len {
14+
buf.push(((*offset + i) % 251) as u8);
15+
}
16+
*offset += len;
17+
buf
18+
}
19+
20+
fn addr() -> String {
21+
test_programs::p3::wasi::cli::environment::get_environment()
22+
.into_iter()
23+
.find_map(|(k, v)| k.eq("HTTP_SERVER").then_some(v))
24+
.unwrap()
25+
}
26+
27+
impl test_programs::p3::exports::wasi::cli::run::Guest for Component {
28+
async fn run() -> Result<(), ()> {
29+
test_chunked_write().await;
30+
Ok(())
31+
}
32+
}
33+
34+
async fn test_chunked_write() {
35+
let headers = Headers::from_list(&[]).unwrap();
36+
let (mut contents_tx, contents_rx) = wit_stream::new();
37+
let (trailers_tx, trailers_rx) = wit_future::new(|| Ok(None));
38+
let (request, transmit) = Request::new(headers, Some(contents_rx), trailers_rx, None);
39+
configure(&request);
40+
41+
let (transmit, written, echoed) = join!(
42+
async { transmit.await },
43+
async {
44+
let mut len = 16;
45+
let mut pos = 0;
46+
loop {
47+
assert!(len <= 128 << 20);
48+
let (result, remaining) = contents_tx.write(bytes(&mut pos, len)).await;
49+
assert_eq!(result, StreamResult::Complete(len - remaining.remaining()));
50+
if remaining.remaining() == 0 {
51+
len = len.checked_mul(2).unwrap();
52+
} else {
53+
pos -= remaining.remaining();
54+
break;
55+
}
56+
}
57+
drop(contents_tx);
58+
_ = trailers_tx.write(Ok(None)).await;
59+
pos
60+
},
61+
async { send_and_collect(request).await },
62+
);
63+
transmit.unwrap();
64+
assert_eq!(echoed, bytes(&mut 0, written));
65+
}
66+
67+
fn configure(request: &Request) {
68+
request.set_method(&Method::Post).unwrap();
69+
request.set_scheme(Some(&Scheme::Http)).unwrap();
70+
request.set_authority(Some(&addr())).unwrap();
71+
request.set_path_with_query(Some("/")).unwrap();
72+
}
73+
74+
async fn send_and_collect(request: Request) -> Vec<u8> {
75+
let response = client::send(request).await.unwrap();
76+
assert_eq!(response.get_status_code(), 200);
77+
let (_, result_rx) = wit_future::new(|| Ok(()));
78+
let (body_rx, trailers_rx) = Response::consume_body(response, result_rx);
79+
let body = body_rx.collect().await;
80+
trailers_rx.await.unwrap();
81+
body
82+
}
83+
84+
fn main() {
85+
unreachable!()
86+
}

crates/wasi-http/src/ctx.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,13 @@ pub trait WasiHttpHooks: Send {
347347
})
348348
}
349349

350+
/// Maximum number of bytes the implementation will copy out of the guest in
351+
/// a single write to an outgoing body's stream.
352+
#[cfg(feature = "p3")]
353+
fn p3_outgoing_body_chunk_size(&mut self) -> usize {
354+
crate::p3::DEFAULT_OUTGOING_BODY_CHUNK_SIZE
355+
}
356+
350357
/// Optional hook to configure the error code for hyper errors.
351358
#[cfg(feature = "p3")]
352359
fn p3_error_from_hyper(&mut self, err: &hyper::Error) -> p3::ErrorCode {

crates/wasi-http/src/p3/body.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ struct LimitedGuestBodyConsumer {
177177
limit: u64,
178178
/// Number of bytes sent
179179
sent: u64,
180+
max_chunk_size: usize,
180181
// `true` when the other side of `contents_tx` was unexpectedly closed
181182
closed: bool,
182183
}
@@ -217,7 +218,8 @@ impl<D> StreamConsumer<D> for LimitedGuestBodyConsumer {
217218
debug_assert!(!self.closed);
218219
let mut src = src.as_direct(store);
219220
let buf = src.remaining();
220-
let n = buf.len();
221+
let n = buf.len().min(self.max_chunk_size);
222+
let buf = &buf[..n];
221223

222224
// Perform `content-length` check early and precompute the next value
223225
let Ok(sent) = n.try_into() else {
@@ -260,7 +262,10 @@ impl<D> StreamConsumer<D> for LimitedGuestBodyConsumer {
260262

261263
/// [StreamConsumer] implementation for bodies originating in the guest without `Content-Length`
262264
/// header set.
263-
struct UnlimitedGuestBodyConsumer(PollSender<Result<Bytes, ErrorCode>>);
265+
struct UnlimitedGuestBodyConsumer {
266+
contents_tx: PollSender<Result<Bytes, ErrorCode>>,
267+
max_chunk_size: usize,
268+
}
264269

265270
impl<D> StreamConsumer<D> for UnlimitedGuestBodyConsumer {
266271
type Item = u8;
@@ -272,13 +277,13 @@ impl<D> StreamConsumer<D> for UnlimitedGuestBodyConsumer {
272277
src: Source<Self::Item>,
273278
finish: bool,
274279
) -> Poll<wasmtime::Result<StreamResult>> {
275-
match self.0.poll_reserve(cx) {
280+
match self.contents_tx.poll_reserve(cx) {
276281
Poll::Ready(Ok(())) => {
277282
let mut src = src.as_direct(store);
278283
let buf = src.remaining();
279-
let n = buf.len();
280-
let buf = Bytes::copy_from_slice(buf);
281-
match self.0.send_item(Ok(buf)) {
284+
let n = buf.len().min(self.max_chunk_size);
285+
let buf = Bytes::copy_from_slice(&buf[..n]);
286+
match self.contents_tx.send_item(Ok(buf)) {
282287
Ok(()) => {
283288
src.mark_read(n);
284289
Poll::Ready(Ok(StreamResult::Completed))
@@ -329,6 +334,11 @@ impl GuestBody {
329334
Ok(())
330335
})?;
331336

337+
let max_chunk_size = getter(store.as_context_mut().data_mut())
338+
.hooks
339+
.p3_outgoing_body_chunk_size()
340+
.max(1);
341+
332342
let contents_rx = if let Some(rx) = contents_rx {
333343
let (http_tx, http_rx) = mpsc::channel(1);
334344
let contents_tx = PollSender::new(http_tx);
@@ -348,12 +358,19 @@ impl GuestBody {
348358
make_error,
349359
limit,
350360
sent: 0,
361+
max_chunk_size,
351362
closed: false,
352363
},
353364
)?;
354365
} else {
355366
_ = result_tx.send(Box::new(result_fut));
356-
rx.pipe(store, UnlimitedGuestBodyConsumer(contents_tx))?;
367+
rx.pipe(
368+
store,
369+
UnlimitedGuestBodyConsumer {
370+
contents_tx,
371+
max_chunk_size,
372+
},
373+
)?;
357374
};
358375
Some(http_rx)
359376
} else {

crates/wasi-http/src/p3/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ mod response;
2020
pub use request::Request;
2121
pub use response::Response;
2222

23+
/// The default value configured for [`WasiHttpHooks::p3_outgoing_body_chunk_size`].
24+
///
25+
/// [`WasiHttpHooks::p3_outgoing_body_chunk_size`]: crate::WasiHttpHooks::p3_outgoing_body_chunk_size
26+
pub const DEFAULT_OUTGOING_BODY_CHUNK_SIZE: usize = 1024 * 1024;
27+
2328
use crate::{FieldMapError, WasiHttp, WasiHttpView};
2429
use bindings::http::{client, types};
2530
use core::ops::Deref;

crates/wasi-http/tests/all/p3/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ async fn run_cli(path: &str, server: &Server) -> wasmtime::Result<()> {
127127
Ctx {
128128
wasi: wasmtime_wasi::WasiCtx::builder()
129129
.env("HTTP_SERVER", server.addr())
130+
.inherit_stdio()
130131
.build(),
131132
..Ctx::new(oneshot::channel().0)
132133
},
@@ -870,3 +871,9 @@ async fn p3_http_empty_frames_interleaved() -> Result<()> {
870871
assert_eq!(collected_body, b"hello world".as_slice());
871872
Ok(())
872873
}
874+
875+
#[test_log::test(tokio::test(flavor = "multi_thread"))]
876+
async fn p3_http_outbound_request_chunk_size() -> Result<()> {
877+
let server = Server::http1(1)?;
878+
run_cli(P3_HTTP_OUTBOUND_REQUEST_CHUNK_SIZE_COMPONENT, &server).await
879+
}

crates/wasi/src/p3/filesystem/host.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,9 @@ impl<D> StreamConsumer<D> for WriteStreamConsumer {
463463
let me = &mut *self;
464464
let task = me.task.get_or_insert_with(|| {
465465
debug_assert!(me.buffer.is_empty());
466-
me.buffer.extend_from_slice(src.remaining());
466+
let remaining = src.remaining();
467+
let n = remaining.len().min(DEFAULT_BUFFER_CAPACITY);
468+
me.buffer.extend_from_slice(&remaining[..n]);
467469
let buf = mem::take(&mut me.buffer);
468470
let file = Arc::clone(me.file.as_file());
469471
let location = me.location;

crates/wasi/tests/all/p3/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,11 @@ async fn p3_file_write_blocking() -> wasmtime::Result<()> {
185185
run_allow_blocking_current_thread(P3_FILE_WRITE_COMPONENT, true).await
186186
}
187187

188+
#[test_log::test(tokio::test(flavor = "multi_thread"))]
189+
async fn p3_file_write_chunked() -> wasmtime::Result<()> {
190+
run(P3_FILE_WRITE_CHUNKED_COMPONENT).await
191+
}
192+
188193
#[test_log::test(tokio::test(flavor = "multi_thread"))]
189194
async fn p3_file_truncation_readonly() -> wasmtime::Result<()> {
190195
run_with_readonly_testfile(P3_FILE_TRUNCATION_READONLY_COMPONENT).await

0 commit comments

Comments
 (0)