Skip to content

Commit 053b31f

Browse files
committed
feat: ensure that guest only returns a limited set of UDFs
1 parent 78c7d9d commit 053b31f

File tree

5 files changed

+63
-1
lines changed

5 files changed

+63
-1
lines changed

guests/evil/src/complex/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use datafusion_expr::{
88

99
pub(crate) mod udf_long_name;
1010
pub(crate) mod udfs_duplicate_names;
11+
pub(crate) mod udfs_many;
1112

1213
/// UDF with a name
1314
#[derive(Debug, PartialEq, Eq, Hash)]
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//! Create many UDFs.
2+
use std::sync::Arc;
3+
4+
use datafusion_common::Result as DataFusionResult;
5+
use datafusion_expr::ScalarUDFImpl;
6+
7+
use crate::complex::NamedUdf;
8+
9+
/// Returns our evil UDFs.
10+
///
11+
/// The passed `source` is ignored.
12+
#[expect(clippy::unnecessary_wraps, reason = "public API through export! macro")]
13+
pub(crate) fn udfs(_source: String) -> DataFusionResult<Vec<Arc<dyn ScalarUDFImpl>>> {
14+
let limit: usize = std::env::var("limit").unwrap().parse().unwrap();
15+
16+
Ok((0..=limit)
17+
.map(|i| Arc::new(NamedUdf(i.to_string())) as _)
18+
.collect())
19+
}

guests/evil/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ impl Evil {
4343
root: Box::new(common::root_empty),
4444
udfs: Box::new(complex::udfs_duplicate_names::udfs),
4545
},
46+
"complex::udfs_many" => Self {
47+
root: Box::new(common::root_empty),
48+
udfs: Box::new(complex::udfs_many::udfs),
49+
},
4650
"env" => Self {
4751
root: Box::new(common::root_empty),
4852
udfs: Box::new(env::udfs),

host/src/lib.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ pub struct WasmPermissions {
274274
/// Trusted data limits.
275275
trusted_data_limits: TrustedDataLimits,
276276

277+
/// Maximum number of UDFs.
278+
max_udfs: usize,
279+
277280
/// Environment variables.
278281
envs: BTreeMap<String, String>,
279282
}
@@ -300,6 +303,7 @@ impl Default for WasmPermissions {
300303
stderr_bytes: 1024, // 1KB
301304
resource_limits: StaticResourceLimits::default(),
302305
trusted_data_limits: TrustedDataLimits::default(),
306+
max_udfs: 20,
303307
envs: BTreeMap::default(),
304308
}
305309
}
@@ -378,6 +382,19 @@ impl WasmPermissions {
378382
}
379383
}
380384

385+
/// Get the maximum number of UDFs that a payload/guest can produce.
386+
pub fn max_udfs(&self) -> usize {
387+
self.max_udfs
388+
}
389+
390+
/// Set the maximum number of UDFs that a payload/guest can produce.
391+
pub fn with_max_udfs(self, limit: usize) -> Self {
392+
Self {
393+
max_udfs: limit,
394+
..self
395+
}
396+
}
397+
381398
/// Add environment variable.
382399
pub fn with_env(mut self, key: String, value: String) -> Self {
383400
self.envs.insert(key, value);
@@ -578,6 +595,13 @@ impl WasmScalarUdf {
578595
)?
579596
.convert_err(permissions.trusted_data_limits.clone())
580597
.context("scalar_udfs")?;
598+
if udf_resources.len() > permissions.max_udfs {
599+
return Err(DataFusionError::ResourcesExhausted(format!(
600+
"guest returned too many UDFs: got={}, limit={}",
601+
udf_resources.len(),
602+
permissions.max_udfs,
603+
)));
604+
}
581605

582606
let store = Arc::new(Mutex::new(store));
583607

host/tests/integration_tests/evil/complex.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use datafusion_udf_wasm_host::conversion::limits::TrustedDataLimits;
1+
use datafusion_udf_wasm_host::{WasmPermissions, conversion::limits::TrustedDataLimits};
22

33
use crate::integration_tests::evil::test_utils::{try_scalar_udfs, try_scalar_udfs_with_env};
44

@@ -31,3 +31,17 @@ async fn test_udfs_duplicate_names() {
3131
err,
3232
@"External error: non-unique UDF name: 'foo'");
3333
}
34+
35+
#[tokio::test]
36+
async fn test_udfs_many() {
37+
let err = try_scalar_udfs_with_env(
38+
"complex::udfs_many",
39+
&[("limit", &WasmPermissions::default().max_udfs().to_string())],
40+
)
41+
.await
42+
.unwrap_err();
43+
44+
insta::assert_snapshot!(
45+
err,
46+
@"Resources exhausted: guest returned too many UDFs: got=21, limit=20");
47+
}

0 commit comments

Comments
 (0)