mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-20 16:02:19 +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::error::JsonResult;
|
||||||
use windmill_common::flow_status::{JobResult, RestartedFrom};
|
use windmill_common::flow_status::{JobResult, RestartedFrom};
|
||||||
use windmill_common::jobs::{
|
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")]
|
#[cfg(feature = "run_inline")]
|
||||||
use windmill_common::jobs::{
|
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_tag_available_for_workspace(&db, &w_id, &run_query.tag, &authed).await?;
|
||||||
check_scopes(&authed, || format!("jobs:run"))?;
|
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;
|
let mut i = 1;
|
||||||
|
|
||||||
if *CLOUD_HOSTED {
|
if *CLOUD_HOSTED {
|
||||||
@@ -6769,6 +6777,14 @@ async fn run_dynamic_select(
|
|||||||
match request.runnable_ref {
|
match request.runnable_ref {
|
||||||
DynamicSelectRunnableRef::Deployed { path, runnable_kind } => match runnable_kind {
|
DynamicSelectRunnableRef::Deployed { path, runnable_kind } => match runnable_kind {
|
||||||
RunnableKind::Script => {
|
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();
|
let mut script_args = request.args.unwrap_or_default();
|
||||||
script_args.insert(
|
script_args.insert(
|
||||||
"_ENTRYPOINT_OVERRIDE".to_string(),
|
"_ENTRYPOINT_OVERRIDE".to_string(),
|
||||||
|
|||||||
@@ -556,6 +556,32 @@ pub struct OnBehalfOf {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub const ENTRYPOINT_OVERRIDE: &str = "_ENTRYPOINT_OVERRIDE";
|
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 LARGE_LOG_THRESHOLD_SIZE: usize = 9000;
|
||||||
pub const EMAIL_ERROR_HANDLER_USER_EMAIL: &str = "email_error_handler@windmill.dev";
|
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 {
|
pub fn generate_dynamic_input_key(workspace_id: &str, path: &str) -> String {
|
||||||
format!("{workspace_id}:{path}")
|
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>>,
|
modules: &Option<std::collections::HashMap<String, ScriptModule>>,
|
||||||
run_inline: bool,
|
run_inline: bool,
|
||||||
) -> error::Result<Box<RawValue>> {
|
) -> 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
|
// Expand WM_INTERNAL_DB markers into real SQL before dispatching
|
||||||
let expanded_code: String;
|
let expanded_code: String;
|
||||||
let mut language = language;
|
let mut language = language;
|
||||||
|
|||||||
Reference in New Issue
Block a user