Skip to content

Commit bb0cf69

Browse files
committed
fix(hu-union): harden wasm plugin loading against untrusted manifests
- sanitize filename-derived plugin stem to prevent work-dir path traversal - bound the rate-tracker channel to cap host memory on high-rate topics - gate declared session opening behind the OpenSession permission - spawn an OS-thread epoch ticker when no Tokio runtime is present so runaway plugins stay preemptible
1 parent 7bd8ec8 commit bb0cf69

2 files changed

Lines changed: 74 additions & 7 deletions

File tree

crates/hiroz-union/src/plugin/wasm/mod.rs

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,14 +225,29 @@ fn shared_wasm_engine() -> Result<Engine> {
225225
// engine that wins the `get_or_init` race gets its ticker spawned.
226226
let candidate = configured_wasm_engine()?;
227227
let engine = SHARED_WASM_ENGINE.get_or_init(|| {
228+
let ticker_engine = candidate.clone();
228229
if let Ok(handle) = tokio::runtime::Handle::try_current() {
229-
let ticker_engine = candidate.clone();
230230
handle.spawn(async move {
231231
loop {
232232
tokio::time::sleep(Duration::from_millis(100)).await;
233233
ticker_engine.increment_epoch();
234234
}
235235
});
236+
} else {
237+
// No Tokio runtime in this context. Epoch interruption would be
238+
// silently disabled (the epoch would never advance, so a runaway
239+
// plugin could not be preempted). Fall back to a dedicated OS
240+
// thread that increments the epoch on the same cadence, so
241+
// preemption keeps working regardless of the caller's runtime.
242+
std::thread::Builder::new()
243+
.name("hu-wasm-epoch".into())
244+
.spawn(move || {
245+
loop {
246+
std::thread::sleep(Duration::from_millis(100));
247+
ticker_engine.increment_epoch();
248+
}
249+
})
250+
.ok();
236251
}
237252
candidate
238253
});
@@ -349,12 +364,36 @@ fn iter_wasm_files() -> impl Iterator<Item = PathBuf> {
349364
.filter(|p| p.extension().and_then(|e| e.to_str()) == Some("wasm"))
350365
}
351366

367+
/// Sanitize a filename-derived plugin stem for safe use as a single path
368+
/// segment. The stem comes straight from an on-disk filename, so a crafted
369+
/// name like `hu-..\.wasm` would otherwise yield a stem of `..` and let the
370+
/// per-plugin work dir escape its base (path traversal). Keep only a safe
371+
/// `[A-Za-z0-9_-]` set (path separators and dots become `_`), and never let
372+
/// the result be empty or a `.`/`..` traversal component.
373+
fn sanitize_plugin_stem(plugin_stem: &str) -> String {
374+
let cleaned: String = plugin_stem
375+
.chars()
376+
.map(|c| {
377+
if c.is_ascii_alphanumeric() || c == '_' || c == '-' {
378+
c
379+
} else {
380+
'_'
381+
}
382+
})
383+
.collect();
384+
if cleaned.is_empty() || cleaned.chars().all(|c| c == '.') {
385+
"unknown".to_string()
386+
} else {
387+
cleaned
388+
}
389+
}
390+
352391
fn plugin_work_dir(plugin_stem: &str) -> PathBuf {
353392
dirs::data_local_dir()
354393
.unwrap_or_else(|| PathBuf::from("."))
355394
.join("hu")
356395
.join("plugin-work")
357-
.join(plugin_stem)
396+
.join(sanitize_plugin_stem(plugin_stem))
358397
}
359398

360399
type StateAndStore = (
@@ -501,6 +540,22 @@ fn open_declared_sessions(
501540
store: &mut Store<PluginState>,
502541
manifest: &hu::plugin::types::PluginManifest,
503542
) -> Result<()> {
543+
// The manifest is untrusted: opening a session triggers an outbound Zenoh
544+
// connection, so it must be gated exactly like the runtime `open_session`
545+
// host call (see host/transport.rs). Refuse to open *any* declared session
546+
// unless the plugin was granted `OpenSession` — otherwise a plugin could
547+
// trigger network connections it never declared a permission for.
548+
if !manifest.sessions.is_empty()
549+
&& !manifest
550+
.required_permissions
551+
.contains(&hu::plugin::types::Permission::OpenSession)
552+
{
553+
anyhow::bail!(
554+
"plugin declares {} session(s) but did not request the OpenSession permission",
555+
manifest.sessions.len()
556+
);
557+
}
558+
504559
for req in &manifest.sessions {
505560
let name = req.name.clone();
506561
let endpoint = req.endpoint.clone();

crates/hiroz-union/src/plugin/wasm/state.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ use crate::core::engine::CoreEngine;
1313
use super::host::hu;
1414
use hu::plugin::types::Permission;
1515

16+
/// Upper bound on buffered rate-tracker arrivals per topic. Caps host memory
17+
/// on high-rate topics; anything beyond this window is stale and dropped.
18+
const RATE_TRACKER_CHANNEL_CAP: usize = 16_384;
19+
1620
// ─── Subscription tracking (ros interface) ────────────────────────────────────
1721

1822
pub(crate) struct SubscriptionData {
@@ -162,7 +166,12 @@ impl PluginState {
162166
let topic_stripped = topic.trim_start_matches('/').to_string();
163167
let ke = format!("{domain_id}/{topic_stripped}/**");
164168
let session = self.engine.session.clone();
165-
let (tx, rx) = flume::unbounded::<(Instant, usize)>();
169+
// Bounded so a high-rate topic (or infrequent `measure` calls that drain
170+
// the tracker slowly) can't grow host memory without bound / OOM. The
171+
// window is trimmed on every measurement anyway, so a backlog beyond this
172+
// cap is already stale; best-effort send drops the newest arrival when
173+
// full rather than blocking the subscriber task.
174+
let (tx, rx) = flume::bounded::<(Instant, usize)>(RATE_TRACKER_CHANNEL_CAP);
166175
let ke_clone = ke.clone();
167176
let handle = tokio::spawn(async move {
168177
let sub = match session.declare_subscriber(&ke_clone).await {
@@ -174,10 +183,13 @@ impl PluginState {
174183
};
175184
while let Ok(sample) = sub.recv_async().await {
176185
let size = sample.payload().to_bytes().len();
177-
// Stop once the tracker (receiver) is gone — otherwise this
178-
// loop spins forever after the RateTrackerData is dropped.
179-
if tx.send((Instant::now(), size)).is_err() {
180-
break;
186+
match tx.try_send((Instant::now(), size)) {
187+
Ok(()) => {}
188+
// Channel full: drop this arrival (best-effort) and keep going.
189+
Err(flume::TrySendError::Full(_)) => {}
190+
// Stop once the tracker (receiver) is gone — otherwise this
191+
// loop spins forever after the RateTrackerData is dropped.
192+
Err(flume::TrySendError::Disconnected(_)) => break,
181193
}
182194
}
183195
});

0 commit comments

Comments
 (0)