diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 231b62e234..4d61df606e 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -33,7 +33,8 @@ use windmill_common::db::UserDbWithAuthed; use windmill_common::error::JsonResult; use windmill_common::flow_status::{JobResult, RestartedFrom}; use windmill_common::jobs::{ - format_completed_job_result, format_result, DynamicInput, ENTRYPOINT_OVERRIDE, + format_completed_job_result, format_result, is_valid_entrypoint_name, DynamicInput, + ENTRYPOINT_OVERRIDE, }; #[cfg(feature = "run_inline")] use windmill_common::jobs::{ @@ -4514,6 +4515,13 @@ pub async fn run_workflow_as_code( check_tag_available_for_workspace(&db, &w_id, &run_query.tag, &authed).await?; check_scopes(&authed, || format!("jobs:run"))?; + if !is_valid_entrypoint_name(&entrypoint) { + return Err(error::Error::BadRequest(format!( + "Invalid entrypoint {entrypoint:?}: must match ^[A-Za-z_][A-Za-z0-9_]*$ \ + (letters, digits and underscores, not starting with a digit)" + ))); + } + let mut i = 1; if *CLOUD_HOSTED { @@ -6769,6 +6777,14 @@ async fn run_dynamic_select( match request.runnable_ref { DynamicSelectRunnableRef::Deployed { path, runnable_kind } => match runnable_kind { RunnableKind::Script => { + if !is_valid_entrypoint_name(&request.entrypoint_function) { + return Err(error::Error::BadRequest(format!( + "Invalid entrypoint_function {:?}: must match \ + ^[A-Za-z_][A-Za-z0-9_]*$ (letters, digits and underscores, \ + not starting with a digit)", + request.entrypoint_function + ))); + } let mut script_args = request.args.unwrap_or_default(); script_args.insert( "_ENTRYPOINT_OVERRIDE".to_string(), diff --git a/backend/windmill-types/src/jobs.rs b/backend/windmill-types/src/jobs.rs index 03e38d9db3..308a326b64 100644 --- a/backend/windmill-types/src/jobs.rs +++ b/backend/windmill-types/src/jobs.rs @@ -556,6 +556,32 @@ pub struct OnBehalfOf { } pub const ENTRYPOINT_OVERRIDE: &str = "_ENTRYPOINT_OVERRIDE"; + +/// The entrypoint override (`_ENTRYPOINT_OVERRIDE` job arg -> +/// `v2_job.script_entrypoint_override`) is interpolated verbatim into +/// generated worker wrappers in a code position (e.g. the NativeTS +/// `import(...).then(m => m.(...))` glue, the bun `Main.(...)` +/// call, the deno `import { } from "./main.ts"` line, the PHP +/// `(...)` call and the Python `inner_script.(**args)` call). +/// A caller only needs `jobs:run` to set it, so it MUST be restricted to a +/// conventional identifier or an attacker who can merely run a deployed +/// script could break out of the call expression into arbitrary +/// worker-process code. This ASCII subset is a valid function name in every +/// language Windmill wraps this way (JS/TS, Python, PHP). +pub fn is_valid_entrypoint_name(name: &str) -> bool { + if name.is_empty() || name.len() > 255 { + return false; + } + let mut chars = name.chars(); + let Some(first) = chars.next() else { + return false; + }; + if !(first.is_ascii_alphabetic() || first == '_') { + return false; + } + chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} + pub const LARGE_LOG_THRESHOLD_SIZE: usize = 9000; pub const EMAIL_ERROR_HANDLER_USER_EMAIL: &str = "email_error_handler@windmill.dev"; @@ -563,3 +589,52 @@ pub const EMAIL_ERROR_HANDLER_USER_EMAIL: &str = "email_error_handler@windmill.d pub fn generate_dynamic_input_key(workspace_id: &str, path: &str) -> String { format!("{workspace_id}:{path}") } + +#[cfg(test)] +mod tests { + use super::is_valid_entrypoint_name; + + #[test] + fn valid_entrypoint_names_are_accepted() { + for name in [ + "main", + "preprocessor", + "my_helper", + "_private", + "fn2", + "a", + "MixedCase", + ] { + assert!(is_valid_entrypoint_name(name), "expected {name:?} valid"); + } + } + + #[test] + fn malicious_entrypoint_names_are_rejected() { + // Regression for GHSA-wxjq-w5pj-jqhx: the entrypoint override is + // interpolated verbatim into a code position of generated worker + // wrappers (e.g. bun `Main.(...)`, nativets + // `m.(...)`, python `inner_script.(**args)`). Any value + // that is not a strict identifier could break out of the call + // expression into attacker-controlled worker code. + for name in [ + "main(); globalThis.x = 1; //", // breaks out of `Main.(...)` + "x); require('child_process').execSync('id'); (", + "1main", // starts with a digit + "my-fn", // hyphen + "my fn", // space + "my.fn", // member access + "$fn", // dollar + "fn\nother", // newline + "fn;other", + "", + ] { + assert!( + !is_valid_entrypoint_name(name), + "expected {name:?} to be rejected" + ); + } + // Over-long names are rejected. + assert!(!is_valid_entrypoint_name(&"a".repeat(256))); + } +} diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index f9305961eb..7ff9823072 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -4436,6 +4436,21 @@ pub async fn run_language_executor( modules: &Option>, run_inline: bool, ) -> error::Result> { + // Defense-in-depth (GHSA-wxjq-w5pj-jqhx): the entrypoint override is + // interpolated verbatim into a code position of the generated language + // wrappers below. It originates from the `_ENTRYPOINT_OVERRIDE` job arg, + // which any caller with `jobs:run` can set on a deployed script, so reject + // anything that is not a strict identifier before it reaches any wrapper. + if let Some(entrypoint) = job.script_entrypoint_override.as_deref() { + if !windmill_common::jobs::is_valid_entrypoint_name(entrypoint) { + return Err(Error::BadRequest(format!( + "Invalid entrypoint override {entrypoint:?}: must match \ + ^[A-Za-z_][A-Za-z0-9_]*$ (letters, digits and underscores, \ + not starting with a digit)" + ))); + } + } + // Expand WM_INTERNAL_DB markers into real SQL before dispatching let expanded_code: String; let mut language = language;