Skip to content

Commit 85348be

Browse files
[46.0.x] Backports for security fixes (#14177)
* 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 * Fix doc link --------- Co-authored-by: Till Schneidereit <till@tillschneidereit.net>
1 parent 4f87260 commit 85348be

11 files changed

Lines changed: 255 additions & 37 deletions

File tree

Cargo.lock

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

Cargo.toml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,11 @@ wasip1 = { version = "1.0.0", default-features = false }
351351

352352
# cap-std family:
353353
target-lexicon = "0.13.3"
354-
cap-std = "3.4.5"
355-
cap-fs-ext = "3.4.5"
356-
cap-net-ext = "3.4.5"
357-
cap-time-ext = "3.4.5"
358-
cap-tempfile = "3.4.5"
354+
cap-std = "3.4.6"
355+
cap-fs-ext = "3.4.6"
356+
cap-net-ext = "3.4.6"
357+
cap-time-ext = "3.4.6"
358+
cap-tempfile = "3.4.6"
359359
fs-set-times = "0.20.3"
360360
system-interface = { version = "0.27.3", features = ["cap_std_impls"] }
361361
io-lifetimes = { version = "2.0.3", 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/p3/body.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ struct LimitedGuestBodyConsumer {
139139
limit: u64,
140140
/// Number of bytes sent
141141
sent: u64,
142+
max_chunk_size: usize,
142143
// `true` when the other side of `contents_tx` was unexpectedly closed
143144
closed: bool,
144145
}
@@ -179,7 +180,8 @@ impl<D> StreamConsumer<D> for LimitedGuestBodyConsumer {
179180
debug_assert!(!self.closed);
180181
let mut src = src.as_direct(store);
181182
let buf = src.remaining();
182-
let n = buf.len();
183+
let n = buf.len().min(self.max_chunk_size);
184+
let buf = &buf[..n];
183185

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

223225
/// [StreamConsumer] implementation for bodies originating in the guest without `Content-Length`
224226
/// header set.
225-
struct UnlimitedGuestBodyConsumer(PollSender<Result<Bytes, ErrorCode>>);
227+
struct UnlimitedGuestBodyConsumer {
228+
contents_tx: PollSender<Result<Bytes, ErrorCode>>,
229+
max_chunk_size: usize,
230+
}
226231

227232
impl<D> StreamConsumer<D> for UnlimitedGuestBodyConsumer {
228233
type Item = u8;
@@ -234,13 +239,13 @@ impl<D> StreamConsumer<D> for UnlimitedGuestBodyConsumer {
234239
src: Source<Self::Item>,
235240
finish: bool,
236241
) -> Poll<wasmtime::Result<StreamResult>> {
237-
match self.0.poll_reserve(cx) {
242+
match self.contents_tx.poll_reserve(cx) {
238243
Poll::Ready(Ok(())) => {
239244
let mut src = src.as_direct(store);
240245
let buf = src.remaining();
241-
let n = buf.len();
242-
let buf = Bytes::copy_from_slice(buf);
243-
match self.0.send_item(Ok(buf)) {
246+
let n = buf.len().min(self.max_chunk_size);
247+
let buf = Bytes::copy_from_slice(&buf[..n]);
248+
match self.contents_tx.send_item(Ok(buf)) {
244249
Ok(()) => {
245250
src.mark_read(n);
246251
Poll::Ready(Ok(StreamResult::Completed))
@@ -283,6 +288,11 @@ impl GuestBody {
283288
},
284289
)?;
285290

291+
let max_chunk_size = getter(store.as_context_mut().data_mut())
292+
.hooks
293+
.p3_outgoing_body_chunk_size()
294+
.max(1);
295+
286296
let contents_rx = if let Some(rx) = contents_rx {
287297
let (http_tx, http_rx) = mpsc::channel(1);
288298
let contents_tx = PollSender::new(http_tx);
@@ -302,12 +312,19 @@ impl GuestBody {
302312
make_error,
303313
limit,
304314
sent: 0,
315+
max_chunk_size,
305316
closed: false,
306317
},
307318
)?;
308319
} else {
309320
_ = result_tx.send(Box::new(result_fut));
310-
rx.pipe(store, UnlimitedGuestBodyConsumer(contents_tx))?;
321+
rx.pipe(
322+
store,
323+
UnlimitedGuestBodyConsumer {
324+
contents_tx,
325+
max_chunk_size,
326+
},
327+
)?;
311328
};
312329
Some(http_rx)
313330
} else {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ pub use request::default_send_request;
2121
pub use request::{Request, RequestOptions};
2222
pub use response::Response;
2323

24+
/// The default value configured for `WasiHttpHooks::p3_outgoing_body_chunk_size`.
25+
pub const DEFAULT_OUTGOING_BODY_CHUNK_SIZE: usize = 1024 * 1024;
26+
2427
use crate::p3::bindings::http::types::ErrorCode;
2528
use crate::{DEFAULT_FORBIDDEN_HEADERS, FieldMapError, WasiHttpCtx};
2629
use bindings::http::{client, types};
@@ -160,6 +163,13 @@ pub trait WasiHttpHooks: Send {
160163
)>,
161164
> + Send,
162165
>;
166+
167+
/// Maximum number of bytes the implementation will copy out of the guest in
168+
/// a single write to an outgoing body's stream.
169+
#[cfg(feature = "p3")]
170+
fn p3_outgoing_body_chunk_size(&mut self) -> usize {
171+
crate::p3::DEFAULT_OUTGOING_BODY_CHUNK_SIZE
172+
}
163173
}
164174

165175
#[cfg(feature = "default-send-request")]

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ async fn run_cli(path: &str, server: &Server) -> wasmtime::Result<()> {
130130
Ctx {
131131
wasi: wasmtime_wasi::WasiCtx::builder()
132132
.env("HTTP_SERVER", server.addr())
133+
.inherit_stdio()
133134
.build(),
134135
..Ctx::new(oneshot::channel().0)
135136
},
@@ -821,3 +822,9 @@ async fn p3_http_empty_frames_interleaved() -> Result<()> {
821822
assert_eq!(collected_body, b"hello world".as_slice());
822823
Ok(())
823824
}
825+
826+
#[test_log::test(tokio::test(flavor = "multi_thread"))]
827+
async fn p3_http_outbound_request_chunk_size() -> Result<()> {
828+
let server = Server::http1(1)?;
829+
run_cli(P3_HTTP_OUTBOUND_REQUEST_CHUNK_SIZE_COMPONENT, &server).await
830+
}

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

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