mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix: validate entrypoint override to prevent worker code injection (GHSA-wxjq-w5pj-jqhx) (#9204)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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(),
|
||||
|
||||
@@ -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.<name>(...))` glue, the bun `Main.<name>(...)`
|
||||
/// call, the deno `import { <name> } from "./main.ts"` line, the PHP
|
||||
/// `<name>(...)` call and the Python `inner_script.<name>(**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.<name>(...)`, nativets
|
||||
// `m.<name>(...)`, python `inner_script.<name>(**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.<name>(...)`
|
||||
"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)));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4436,6 +4436,21 @@ pub async fn run_language_executor(
|
||||
modules: &Option<std::collections::HashMap<String, ScriptModule>>,
|
||||
run_inline: bool,
|
||||
) -> error::Result<Box<RawValue>> {
|
||||
// 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;
|
||||
|
||||
Reference in New Issue
Block a user