From 5c50c5567ac3b6c8ceaa1a9a5ccb00f181dee2a2 Mon Sep 17 00:00:00 2001 From: TheHypnoo Date: Sat, 13 Jun 2026 10:58:22 +0200 Subject: [PATCH] fix(link): validate pkg_config names before invoking pkg-config --- crates/perry/src/commands/compile/link/mod.rs | 3 + .../src/commands/compile/link/pkg_config.rs | 79 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 crates/perry/src/commands/compile/link/pkg_config.rs diff --git a/crates/perry/src/commands/compile/link/mod.rs b/crates/perry/src/commands/compile/link/mod.rs index b0416a2a71..242aee8474 100644 --- a/crates/perry/src/commands/compile/link/mod.rs +++ b/crates/perry/src/commands/compile/link/mod.rs @@ -41,6 +41,7 @@ use super::{ mod link_cache; mod native_features; +mod pkg_config; mod platform_cmd; mod windows_link; @@ -1670,6 +1671,7 @@ pub(super) fn build_and_run_link( // Add pkg-config libraries for pkg in &target_config.pkg_config { + pkg_config::validate_pkg_config_name(pkg)?; if let Ok(output) = Command::new("pkg-config").args(["--libs", pkg]).output() { if output.status.success() { let libs = String::from_utf8_lossy(&output.stdout); @@ -1737,6 +1739,7 @@ pub(super) fn build_and_run_link( } } for pkg in &backend.pkg_config { + pkg_config::validate_pkg_config_name(pkg)?; if let Ok(output) = Command::new("pkg-config").args(["--libs", pkg]).output() { if output.status.success() { let libs = String::from_utf8_lossy(&output.stdout); diff --git a/crates/perry/src/commands/compile/link/pkg_config.rs b/crates/perry/src/commands/compile/link/pkg_config.rs new file mode 100644 index 0000000000..5574d2b322 --- /dev/null +++ b/crates/perry/src/commands/compile/link/pkg_config.rs @@ -0,0 +1,79 @@ +//! Validation for user-supplied `pkg-config` package names. +//! +//! `pkg_config` entries come from the user's `package.json` +//! (`perry.nativeLibrary…`) and are passed to `pkg-config --libs `. +//! No shell is involved, so the risk is argument-confusion (e.g. a leading +//! `-` parsed as a flag) rather than shell injection — but validating with a +//! conservative allowlist removes that surface and turns a confusing link +//! failure into an actionable error naming the offending entry (#5068). + +use anyhow::{anyhow, Result}; + +/// Accept a pkg-config package name only if it is a plausible identifier: +/// non-empty, no leading `-` (which `pkg-config` would parse as a flag), and +/// composed solely of characters real package names use. +pub(super) fn validate_pkg_config_name(name: &str) -> Result<()> { + if name.is_empty() { + return Err(anyhow!( + "invalid pkg-config package name: empty string in perry.nativeLibrary pkg_config" + )); + } + if name.starts_with('-') { + return Err(anyhow!( + "invalid pkg-config package name {name:?}: must not start with '-' \ + (would be parsed as a pkg-config flag)" + )); + } + let ok = name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '/' | '@' | '+' | '-')); + if !ok { + return Err(anyhow!( + "invalid pkg-config package name {name:?}: only ASCII alphanumerics and \ + the characters . _ / @ + - are allowed" + )); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_real_package_names() { + for name in ["openssl", "libssl", "gtk+-3.0", "glib-2.0", "sdl2", "zlib"] { + assert!( + validate_pkg_config_name(name).is_ok(), + "{name} should be accepted" + ); + } + } + + #[test] + fn rejects_empty() { + assert!(validate_pkg_config_name("").is_err()); + } + + #[test] + fn rejects_leading_dash_flag() { + assert!(validate_pkg_config_name("--modversion").is_err()); + assert!(validate_pkg_config_name("-lfoo").is_err()); + } + + #[test] + fn rejects_whitespace_and_metacharacters() { + for bad in [ + "openssl evil", + "openssl;rm", + "open$ssl", + "open`ssl`", + "open\nssl", + ] { + assert!( + validate_pkg_config_name(bad).is_err(), + "{bad:?} should be rejected" + ); + } + } +}