Skip to content

Commit b0bdcf8

Browse files
authored
[wasi-sockets] disallow sending unpermitted datagrams (#12641)
In #12629, I removed the enforcement of `outgoing-datagram-stream.check-send` permits, which turned out to be incorrect according to [the spec]. This restores the enforcement to conform to the spec while still fixing the issue addressed by the original PR. [the spec]: https://github.com/WebAssembly/WASI/blob/41d0de898ec7568e032129156a3bc567d7ebea5d/proposals/sockets/wit/udp.wit#L256C1-L257C111
1 parent 12e29c2 commit b0bdcf8

5 files changed

Lines changed: 81 additions & 14 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
use test_programs::wasi::sockets::network::{IpAddress, IpAddressFamily, IpSocketAddress, Network};
2+
use test_programs::wasi::sockets::udp::{OutgoingDatagram, UdpSocket};
3+
4+
/// `outgoing-datagram-stream.send` should trap we attempt to send more
5+
/// datagrams than `check-send` gave permission for.
6+
fn main() {
7+
let net = Network::default();
8+
let family = IpAddressFamily::Ipv4;
9+
let unspecified_port = IpSocketAddress::new(IpAddress::new_loopback(family), 0);
10+
let remote = IpSocketAddress::new(IpAddress::new_loopback(family), 4320);
11+
12+
let client = UdpSocket::new(family).unwrap();
13+
client.blocking_bind(&net, unspecified_port).unwrap();
14+
15+
let (_, tx) = client.stream(Some(remote)).unwrap();
16+
assert_eq!(client.remote_address(), Ok(remote));
17+
18+
tx.subscribe().block();
19+
20+
let permits = tx.check_send().unwrap();
21+
assert!(permits > 0);
22+
// This should trap according to the `wasi-sockets` spec since we're trying
23+
// to send more than `check_send` gave permission for:
24+
tx.send(
25+
&std::iter::repeat_with(|| OutgoingDatagram {
26+
data: b"hello".into(),
27+
remote_address: None,
28+
})
29+
.take(usize::try_from(permits + 1).unwrap())
30+
.collect::<Vec<_>>(),
31+
)
32+
.unwrap();
33+
unreachable!("attempt to send excess datagrams should have trapped");
34+
}

crates/wasi/src/p2/host/udp.rs

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ impl udp::HostUdpSocket for WasiSocketsCtxView<'_> {
8585
remote_address,
8686
family: socket.address_family(),
8787
socket_addr_check: socket.socket_addr_check().cloned(),
88+
check_send_permit_count: 0,
8889
};
8990

9091
Ok((
@@ -264,20 +265,22 @@ impl udp::HostOutgoingDatagramStream for WasiSocketsCtxView<'_> {
264265
fn check_send(&mut self, this: Resource<udp::OutgoingDatagramStream>) -> SocketResult<u64> {
265266
let stream = self.table.get_mut(&this)?;
266267

267-
Ok(
268-
if let Poll::Ready(_) =
269-
pin!(stream.inner.ready(Interest::WRITABLE.add(Interest::ERROR)))
270-
.poll(&mut Context::from_waker(Waker::noop()))
271-
{
272-
// We don't know how many Tokio will accept, so we make up a
273-
// reasonable number here. If we're wrong and `send` returns
274-
// `Ok(0)`, the guest will just have to deal with that, e.g. by
275-
// looping or returning `EWOULDBLOCK`.
276-
16
277-
} else {
278-
0
279-
},
280-
)
268+
let count = if let Poll::Ready(_) =
269+
pin!(stream.inner.ready(Interest::WRITABLE.add(Interest::ERROR)))
270+
.poll(&mut Context::from_waker(Waker::noop()))
271+
{
272+
// We don't know how many Tokio will accept, so we make up a
273+
// reasonable number here. If we're wrong and `send` returns
274+
// `Ok(0)`, the guest will just have to deal with that, e.g. by
275+
// looping or returning `EWOULDBLOCK`.
276+
16
277+
} else {
278+
0
279+
};
280+
281+
stream.check_send_permit_count = count;
282+
283+
Ok(count.try_into().unwrap())
281284
}
282285

283286
async fn send(
@@ -331,6 +334,14 @@ impl udp::HostOutgoingDatagramStream for WasiSocketsCtxView<'_> {
331334
return Ok(0);
332335
}
333336

337+
if datagrams.len() > stream.check_send_permit_count {
338+
return Err(SocketError::trap(wasmtime::format_err!(
339+
"unpermitted: argument exceeds permitted size"
340+
)));
341+
}
342+
343+
stream.check_send_permit_count -= datagrams.len();
344+
334345
let mut count = 0;
335346

336347
for datagram in datagrams {

crates/wasi/src/p2/udp.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,8 @@ pub struct OutgoingDatagramStream {
2020

2121
/// The check of allowed addresses
2222
pub(crate) socket_addr_check: Option<SocketAddrCheck>,
23+
24+
/// Remaining number of datagrams permitted by most recent `check-send`
25+
/// call.
26+
pub(crate) check_send_permit_count: usize,
2327
}

crates/wasi/tests/all/p2/async_.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,16 @@ async fn p2_adapter_badfd() {
361361
async fn p2_file_read_write() {
362362
run(P2_FILE_READ_WRITE_COMPONENT, false).await.unwrap()
363363
}
364+
#[test_log::test(tokio::test(flavor = "multi_thread"))]
365+
async fn p2_udp_send_too_much() {
366+
let e = run(P2_UDP_SEND_TOO_MUCH_COMPONENT, false)
367+
.await
368+
.unwrap_err();
369+
assert_eq!(
370+
format!("{}", e.source().expect("trap source")),
371+
"unpermitted: argument exceeds permitted size"
372+
)
373+
}
364374
#[expect(
365375
dead_code,
366376
reason = "tested in the wasi-cli crate, satisfying foreach_api! macro"

crates/wasi/tests/all/p2/sync.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,14 @@ fn p2_adapter_badfd() {
343343
fn p2_file_read_write() {
344344
run(P2_FILE_READ_WRITE_COMPONENT, false).unwrap()
345345
}
346+
#[test_log::test]
347+
fn p2_udp_send_too_much() {
348+
let e = run(P2_UDP_SEND_TOO_MUCH_COMPONENT, false).unwrap_err();
349+
assert_eq!(
350+
format!("{}", e.source().expect("trap source")),
351+
"unpermitted: argument exceeds permitted size"
352+
)
353+
}
346354
#[expect(
347355
dead_code,
348356
reason = "tested in the wasi-cli crate, satisfying foreach_api! macro"

0 commit comments

Comments
 (0)