From 33fd0b5bef48316b6610e2c44a2778ef6a5be74a Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 12:53:24 -0300 Subject: [PATCH 1/4] Judge server templates: inline scripts, template code and Node views JevGate never read server templates, and they held most of the documented vulnerabilities it missed in the intentionally vulnerable apps. - An ERB, EJS, JSP, Handlebars, Mustache, Nunjucks, Twig, Jinja or Go template, or HTML under templates/, views/, layouts/, partials/ or includes/, is parsed as its inline scripts with its tags blanked, and its top-level script is judged like a PHP page script. Its requests say the code runs in the visitor's browser. - Its `template code` is one more unit: tags that write request, cookie, session or signed-in-user data unescaped (judged by injection), and every scriptlet and declaration of a JSP page once one reads the request (judged by every security rule). - A Node handler that renders a view by name is sent the view's unescaped lines, as Django views are, and its questions name such templates. A template holding neither inline scripts nor template code is not selected. On the corpus, 24 documented vulnerabilities are found that no rule read (RailsGoat's and DVNA's XSS, DVGA's paste-page XSS, DVJA's JSP XSS, JavaVulnerableLab's JSP-only injections, traversal, SSRF and leaked stack traces); 42 of 49 labeled reviews and 16 of 25 considers in templates were right. Requests outside templates and render calls are unchanged on the other 101 projects. --- CHANGELOG.md | 2 + README.md | 2 +- docs/classification-cascade.md | 24 +++- site/src/how-it-works.md | 24 +++- site/src/languages.md | 3 +- site/src/what-it-finds.md | 2 +- src/analysis/django/templates.rs | 24 +++- src/analysis/mod.rs | 2 + src/analysis/sites.rs | 26 ++++ src/analysis/template_code.rs | 199 +++++++++++++++++++++++++++++++ src/analysis/units/mod.rs | 43 +++++-- src/analysis/views.rs | 101 ++++++++++++++++ src/catalog.rs | 2 +- src/components.rs | 102 +++++++++++++++- src/file_kind.rs | 4 + src/inventory/django.rs | 37 +++--- src/inventory/mod.rs | 7 +- src/syntax.rs | 62 +++++++++- src/units/plan/file.rs | 34 +++--- src/units/plan/security_units.rs | 94 ++++++++++----- src/units/questions/mod.rs | 16 ++- src/units/questions/security.rs | 20 ++++ src/units/questions/settle.rs | 20 +++- src/units/security.rs | 120 +++++++++++++++---- src/units/tests/security.rs | 112 ++++++++++++++++- 25 files changed, 966 insertions(+), 116 deletions(-) create mode 100644 src/analysis/template_code.rs create mode 100644 src/analysis/views.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 16615f8..5f10ab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Notable changes to JevGate. Versions follow [Semantic Versioning](https://semver Measured on 103 pinned projects (24 new open-source ones of kinds not tried before, among them intentionally vulnerable Rails, Node, GraphQL, C# and Java apps, a Deno framework, a WordPress plugin, a cookiecutter template and projects in Kotlin, Swift, Elixir and C, and 8 more of the maintainer's own), with findings labeled by hand: on the 70 labeled projects JevGate was tuned on, 75% of reviews were right against 69% with 0.20.0 (136 wrong reviews against 192), and 72% of considers against 65% (254 wrong considers against 354); on 11 held-out projects, 61% of reviews against 57%, and 56% of considers against 54%. Undecided units went from 2.2% to 1.5% of judged units. +- Server templates: ERB, EJS, JSP, Handlebars, Mustache, Nunjucks, Twig, Jinja and Go templates, and HTML under `templates/`, `views/`, `layouts/`, `partials/` or `includes/`, are judged. Their inline `\n" + )); + assert!(!inline_scripts( + "

Hi

\n\n" + )); + assert!(!inline_scripts( + "" + )); + assert!(!inline_scripts("

{{ name }}

")); + } +} diff --git a/src/file_kind.rs b/src/file_kind.rs index e2fae04..04a70f0 100644 --- a/src/file_kind.rs +++ b/src/file_kind.rs @@ -132,6 +132,10 @@ pub fn unsent(path: &Path, named_source: &str, detail: &str) -> Classification { } pub fn language(path: &Path) -> &'static str { + if crate::components::server_template(path) { + // Only its inline scripts are parsed and judged. + return "JavaScript"; + } match extension(path).as_str() { "rs" => "Rust", "py" => "Python", diff --git a/src/inventory/django.rs b/src/inventory/django.rs index ba9ff28..fe5a979 100644 --- a/src/inventory/django.rs +++ b/src/inventory/django.rs @@ -64,12 +64,15 @@ pub(super) fn unescaped_templates( boundary: &Boundary, inputs: &mut [Input], ) { + // Python views name templates as `blog/post.html`; a Node handler + // renders a view by name, as in `res.render('app/products', …)`. let candidate = |input: &Input| { - input.result.path.extension().is_some_and(|e| e == "py") - && input - .source - .as_deref() - .is_some_and(|source| source.contains(".html")) + let source = input.source.as_deref().unwrap_or(""); + match input.result.path.extension().and_then(|e| e.to_str()) { + Some("py") => source.contains(".html"), + Some("js" | "mjs" | "cjs" | "ts" | "mts" | "cts") => source.contains(".render("), + _ => false, + } }; if !inputs.iter().any(candidate) { return; @@ -80,9 +83,10 @@ pub(super) fn unescaped_templates( let Ok(relative) = &crate::discovery::relative(path, &context.root) else { continue; }; - let Some(name) = crate::analysis::django::template_name(relative) else { + let django = crate::analysis::django::template_name(relative); + if django.is_none() && crate::analysis::views::view_name(relative).is_none() { continue; - }; + } if !entry.file_type().is_some_and(|t| t.is_file()) || !boundary.permits(relative) || std::fs::metadata(path) @@ -93,13 +97,18 @@ pub(super) fn unescaped_templates( let Ok(text) = std::fs::read_to_string(path) else { continue; }; - let unescaped = crate::analysis::django::unescaped_lines(&text); - if !unescaped.is_empty() { - templates.push(crate::analysis::django::Template { - name, - path: relative.to_path_buf(), - unescaped, - }); + match django { + Some(name) => { + let unescaped = crate::analysis::django::unescaped_lines(&text); + if !unescaped.is_empty() { + templates.push(crate::analysis::django::Template { + name, + path: relative.to_path_buf(), + unescaped, + }); + } + } + None => templates.extend(crate::analysis::views::view(relative, &text)), } } templates.sort_by(|a, b| a.path.cmp(&b.path)); diff --git a/src/inventory/mod.rs b/src/inventory/mod.rs index a2b991e..56a6664 100644 --- a/src/inventory/mod.rs +++ b/src/inventory/mod.rs @@ -143,7 +143,12 @@ fn source_paths( continue; } let relative = &discovery::relative(path, &context.root)?; - if discovery::source(relative, &args.source_extension) + // A server template counts only for its inline scripts and the code + // that reads client data. + let template = crate::components::server_template(relative) + && std::fs::read_to_string(path) + .is_ok_and(|text| crate::components::judged(relative, &text)); + if (discovery::source(relative, &args.source_extension) || template) && selected(relative) && boundary.permits(relative) && super::context::ensure_visible_path(relative).is_ok() diff --git a/src/syntax.rs b/src/syntax.rs index ce03bb6..c6437b6 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -98,14 +98,20 @@ fn extension(path: &Path) -> &str { /// Whether a parser supports this file's language. pub(crate) fn supported(path: &Path) -> bool { - grammar(path).is_some() || crate::components::FORMATS.contains(&extension(path)) + grammar(path).is_some() + || crate::components::FORMATS.contains(&extension(path)) + || crate::components::server_template(path) } pub(crate) fn parse(path: &Path, source: &str) -> Result> { let extension = extension(path); + let server_template = crate::components::server_template(path); let (language, scripts) = if crate::components::FORMATS.contains(&extension) { let (scripts, language) = crate::components::scripts(extension, source); (language, Some(scripts)) + } else if server_template { + let (scripts, language) = crate::components::scripts("html", source); + (language, Some(without_tags(&scripts, true))) } else if let Some(language) = grammar(path) { ( language, @@ -136,7 +142,7 @@ pub(crate) fn parse(path: &Path, source: &str) -> Result> { }; // Whether errors are tolerable depends on the path, not only the source. ensure!( - if template(path, source) { + if template(path, source) && !server_template { !tree.root_node().has_error() } else { tolerable(tree.root_node(), source.len()) @@ -178,14 +184,31 @@ fn project_template(path: &Path) -> bool { /// that byte offsets and lines stay the file's: `from {{ slug }}.users /// import User` reads as an import, and both branches of an `{% if %}` stay. fn without_jinja(source: &str) -> String { + without_tags(source, false) +} + +/// Jinja's tags blanked as `without_jinja` does, and with `server`, a server +/// template's as well: Handlebars' `{{{ … }}}` and each ERB, EJS or JSP +/// `<%= … %>` or `<%- … %>` read as a name, other `<% … %>` tags blanked. +fn without_tags(source: &str, server: bool) -> String { let bytes = source.as_bytes(); let mut out = bytes.to_vec(); let mut at = 0; while at + 1 < bytes.len() { + let next = bytes.get(at + 2).copied(); let (close, fill) = match (bytes[at], bytes[at + 1]) { (b'{', b'%') => ("%}", b' '), (b'{', b'#') => ("#}", b' '), + (b'{', b'{') if server && next == Some(b'{') => ("}}}", b'_'), (b'{', b'{') => ("}}", b'_'), + (b'<', b'%') if server => ( + "%>", + if matches!(next, Some(b'=' | b'-')) { + b'_' + } else { + b' ' + }, + ), _ => { at += 1; continue; @@ -194,7 +217,7 @@ fn without_jinja(source: &str) -> String { let Some(length) = source[at + 2..].find(close) else { break; }; - let end = at + 2 + length + 2; + let end = at + 2 + length + close.len(); for byte in &mut out[at..end] { if *byte != b'\n' { *byte = fill; @@ -298,6 +321,39 @@ mod tests { ); } + #[test] + fn a_server_template_parses_as_its_inline_scripts_with_its_tags_blanked() { + let erb = "

<%= @title %>

\n<% if admin? %>

Admin

<% end %>\n\n"; + let (masked, _) = crate::components::scripts("html", erb); + let blanked = without_tags(&masked, true); + assert_eq!(blanked.len(), erb.len()); + assert_eq!(blanked.lines().count(), erb.lines().count()); + assert!(!blanked.contains("

") && !blanked.contains("<%")); + assert!(blanked.contains(&format!( + "var tags = {};", + "_".repeat("<%== @tags.to_json %>".len()) + ))); + // Handlebars' triple stash and Jinja's tags, in a template directory. + let jinja = "{% extends 'base.html' %}\n\n"; + for (path, source, function) in [ + ("app/views/sessions/new.html.erb", erb, ("greet", 6)), + ("server/templates/profile.html", jinja, ("show", 5)), + ] { + let path = Path::new(path); + assert!( + !parse(path, source) + .unwrap() + .unwrap() + .root_node() + .has_error() + ); + assert_eq!( + collect(path, source, Path::new(".")).unwrap().1, + vec![(function.0.into(), function.1)] + ); + } + } + #[test] fn component_scripts_parse_in_place() { let astro = "---\nimport Layout from '../layouts/Layout.astro'\nconst posts = await getPosts()\nfunction title(p) { return p.data.title }\n---\n{posts.map(p => {title(p)})}\n\n"; diff --git a/src/units/plan/file.rs b/src/units/plan/file.rs index b28a812..18a7788 100644 --- a/src/units/plan/file.rs +++ b/src/units/plan/file.rs @@ -125,20 +125,26 @@ fn file_context<'a>( source_hash: &input.result.source_hash, model: args.model(), budget, - framework: crate::units::nextjs::describe( - &input.result.path, - input.source.as_deref().unwrap_or(""), - input.package.as_ref(), - ) - .or_else(|| crate::units::sveltekit::describe(&input.result.path, input.package.as_ref())) - .or_else(|| { - crate::units::graphql::describe( - &input.result.path, - input.source.as_deref().unwrap_or(""), - ) - .or_else(|| crate::units::client_app::describe(input.package.as_ref())) - .map(str::to_string) - }), + framework: crate::components::server_template(&input.result.path) + .then(|| crate::components::TEMPLATE_SCRIPT.to_string()) + .or_else(|| { + crate::units::nextjs::describe( + &input.result.path, + input.source.as_deref().unwrap_or(""), + input.package.as_ref(), + ) + }) + .or_else(|| { + crate::units::sveltekit::describe(&input.result.path, input.package.as_ref()) + }) + .or_else(|| { + crate::units::graphql::describe( + &input.result.path, + input.source.as_deref().unwrap_or(""), + ) + .or_else(|| crate::units::client_app::describe(input.package.as_ref())) + .map(str::to_string) + }), } } diff --git a/src/units/plan/security_units.rs b/src/units/plan/security_units.rs index 7690191..5afb45d 100644 --- a/src/units/plan/security_units.rs +++ b/src/units/plan/security_units.rs @@ -38,29 +38,7 @@ pub(super) fn plan_security( .units .iter() .filter(|u| u.callable() && outside_tests(u.line)) - .map(|unit| { - let callers = if rules.contains(&catalog::INJECTION) { - callers_of(scope, &shared.links, context.owner, unit) - } else { - Vec::new() - }; - let mut subject = security::function_subject( - context, - unit, - callers, - &shared.enums, - &shared.constants, - ); - if rules.contains(&catalog::SENSITIVE_DATA) { - subject.callee_errors = callee_errors(scope, &shared.links, context.owner, unit); - } - subject.django = parsed.django; - subject.test_path = test_path; - if parsed.django { - django_evidence(scope, context, parsed, unit, rules, &mut subject); - } - subject - }) + .map(|unit| function_subject(scope, shared, context, unit, rules)) .collect(); let mut setup = security::setup_subject(context, &parsed.setup, &shared.constants) .filter(|_| parsed.setup.statements.iter().all(|s| outside_tests(s.1))); @@ -96,6 +74,56 @@ pub(super) fn plan_security( file, requests, ); + // A server template's code that reads client data: a JSP page's + // scriptlets are judged like a PHP page script, by every rule; other + // templates' code is tags that write a value unescaped, which injection + // judges by where the value comes from. Asked whether they turn off + // escaping, each `raw` or `html_safe` tag of RailsGoat's views said yes, + // even around a user's numeric id. + if let Some(code) = security::template_subject(context, &parsed.template_code) { + let jsp = matches!( + context.path.extension().and_then(|e| e.to_str()), + Some("jsp" | "jspf") + ); + let judged: Vec<&'static str> = rules + .iter() + .copied() + .filter(|rule| jsp || *rule == catalog::INJECTION) + .collect(); + security::plan(context, &[code], None, &judged, false, file, requests); + } +} + +/// A function with the evidence its enabled rules need: callers for +/// injection, the errors its callees create for sensitive data, Django's +/// facts, and the templates it renders. +fn function_subject<'a>( + scope: &'a Scope<'_>, + shared: &'a Shared<'_>, + context: &FileContext<'_>, + unit: &'a Unit, + rules: &[&'static str], +) -> security::Subject<'a> { + let parsed = &scope.units[&context.owner]; + let callers = if rules.contains(&catalog::INJECTION) { + callers_of(scope, &shared.links, context.owner, unit) + } else { + Vec::new() + }; + let mut subject = + security::function_subject(context, unit, callers, &shared.enums, &shared.constants); + if rules.contains(&catalog::SENSITIVE_DATA) { + subject.callee_errors = callee_errors(scope, &shared.links, context.owner, unit); + } + subject.django = parsed.django; + subject.test_path = scope.inputs[context.owner].result.role == "test"; + if parsed.django { + django_evidence(scope, context, parsed, unit, &mut subject); + } + if rules.contains(&catalog::INJECTION) { + rendered_templates(scope, context, &mut subject); + } + subject } /// Module constants shown with one function, at most. @@ -111,7 +139,6 @@ fn django_evidence( context: &FileContext<'_>, parsed: &FileUnits, unit: &Unit, - rules: &[&'static str], subject: &mut security::Subject<'_>, ) { if let Some(command) = crate::analysis::django::management_command(context.path) { @@ -136,6 +163,16 @@ fn django_evidence( serde_json::json!(routes), ); } +} + +/// The templates a function renders by name that write values without +/// escaping, with those lines: the markup a Django view's or a Node +/// handler's values reach is written there, not in the function. +fn rendered_templates( + scope: &Scope<'_>, + context: &FileContext<'_>, + subject: &mut security::Subject<'_>, +) { let templates: Vec = crate::analysis::django::rendered( subject.source.as_str(), &scope.inputs[context.owner].templates, @@ -149,11 +186,10 @@ fn django_evidence( }) }) .collect(); - if rules.contains(&catalog::INJECTION) && !templates.is_empty() { - subject.evidence.insert( - "templates_it_renders_that_write_values_without_escaping".into(), - serde_json::json!(templates), - ); + if !templates.is_empty() { + subject + .evidence + .insert(security::RENDERED.into(), serde_json::json!(templates)); } } diff --git a/src/units/questions/mod.rs b/src/units/questions/mod.rs index 90130ec..7f061c8 100644 --- a/src/units/questions/mod.rs +++ b/src/units/questions/mod.rs @@ -166,8 +166,9 @@ mod tests { security_url_parts("function.source", true), security_redirect_target("function.source", false), security_redirect_target("function.source", true), - security_markup_output("function.source", false), - security_markup_output("function.source", true), + security_markup_output("function.source", false, false), + security_markup_output("function.source", true, false), + security_markup_output("function.source", false, true), security_markup_parts("function.source", false), security_markup_parts("function.source", true), security_path_parts("function.source"), @@ -187,9 +188,11 @@ mod tests { all.extend(security_checks()); for django in [false, true] { all.extend([ - security_interpreted("function.source", django, None, false), - security_interpreted("function.source", django, Some("pickle"), false), - security_interpreted("function.source", django, Some("pickle"), true), + security_interpreted("function.source", django, false, None, false), + security_interpreted("function.source", django, false, Some("pickle"), false), + security_interpreted("function.source", django, false, Some("pickle"), true), + security_interpreted("function.source", django, true, None, false), + security_interpreted("function.source", django, true, Some("pickle"), false), security_resource("function.source", django), security_error_details("function.source", django), security_weakened("function.source", django), @@ -201,7 +204,7 @@ mod tests { for (id, mut body) in [ ( "interpreted", - security_interpreted("function.source", false, None, false), + security_interpreted("function.source", false, false, None, false), ), ("resource", security_resource("function.source", false)), ( @@ -222,6 +225,7 @@ mod tests { .chain(&WEAK_SETTINGS) .chain(&EXPOSURES) .chain(&DJANGO_VARIANTS) + .chain([&VIEW_MARKUP]) .chain(&DJANGO_UNHANDLED) .chain(&DJANGO_SETTINGS) .chain(&DJANGO_EXPOSURES) diff --git a/src/units/questions/security.rs b/src/units/questions/security.rs index e8f922c..a2ec26d 100644 --- a/src/units/questions/security.rs +++ b/src/units/questions/security.rs @@ -16,6 +16,7 @@ use serde_json::{Value, json}; pub fn security_interpreted( code: &str, django: bool, + rendered: bool, deserializers: Option<&str>, xml: bool, ) -> Value { @@ -33,11 +34,16 @@ pub fn security_interpreted( yes.push_str(", marked as safe markup or passed to a template that writes it unescaped, or loaded with pickle or a similar deserializer"); no.push_str(" data is parsed only as JSON or another data-only format;"); } else if let Some(names) = deserializers { + if rendered { + yes.push_str(", or passed to a template that writes it unescaped"); + } question.push_str(", or load it with a deserializer that can build any object"); yes.push_str(&format!( ", or loaded with a deserializer that can build any object or run code, such as {names}" )); no.push_str(" data is parsed only as JSON or another data-only format;"); + } else if rendered { + yes.push_str(", or passed to a template that writes it unescaped"); } if xml { // Both clauses would make the question too long to read as one. @@ -415,6 +421,20 @@ const DJANGO_MARKUP: Check = Check { ], }; +/// The markup check of a function outside Django that renders a template +/// writing values without escaping: DVNA's product search hands the +/// request's search term to `views/app/products.ejs`, which writes it with +/// `<%- … %>`, and the function alone read as building no markup. +pub const VIEW_MARKUP: Check = Check { + id: "markup", + question: "Does `{code}` put a variable into HTML or SVG markup without escaping it, itself or through a template it renders?", + yes: "A variable is joined into HTML or SVG text, or passed to a template that writes it without escaping, such as with EJS `<%- … %>`, Handlebars `{{{ … }}}` or a `|safe` filter, without an escaping function.", + no: "Values go through an escaping function or a template that escapes them, or it builds no markup.", + no_examples: &[ + "A template rendered with the variable, when the template writes that value with an escaping tag, such as EJS `<%= … %>` or Handlebars `{{ … }}`", + ], +}; + /// Specific weak settings, asked when the broad presence question is not clear. /// Turning off output escaping had no check: NodeGoat's `autoescape: false` /// and RailsGoat's `escape_html_entities_in_json = false` were at most notes. diff --git a/src/units/questions/settle.rs b/src/units/questions/settle.rs index c406dff..9e4db02 100644 --- a/src/units/questions/settle.rs +++ b/src/units/questions/settle.rs @@ -116,8 +116,24 @@ pub const INERT_MARKUP: [&str; 3] = ["escaped", "text", "none"]; /// into markup without escaping". A Django view is asked what it sends /// back: views that only redirect or render a template split on the markup /// check, since the variables they pass on end up in a page, and a template -/// escapes them unless it writes one with `|safe`. -pub fn security_markup_output(code: &str, django: bool) -> Value { +/// escapes them unless it writes one with `|safe`. A function elsewhere +/// that renders a template writing values unescaped is asked the same way. +pub fn security_markup_output(code: &str, django: bool, rendered: bool) -> Value { + if rendered && !django { + return json!({ + "type": "choice", + "instructions": { + "question": format!("What does `{code}` send back to the client, and how are the variables in it rendered?"), + "note": EVIDENCE, + }, + "criteria": { + "escaped": "A page rendered from a template that writes each value it is given with an escaping tag, such as EJS `<%= … %>` or Handlebars `{{ … }}`, or HTML built with an escaping function.", + "text": "It is never rendered as HTML: JSON, a file download or plain text.", + "raw": "HTML it builds from variables as text itself, or a template that writes a value it is given without escaping, such as with EJS `<%- … %>`, Handlebars `{{{ … }}}` or a `|safe` filter.", + "none": "No markup with variables: it only redirects, or sends nothing to a client itself.", + }, + }); + } if django { return json!({ "type": "choice", diff --git a/src/units/security.rs b/src/units/security.rs index de6dbc6..90097ef 100644 --- a/src/units/security.rs +++ b/src/units/security.rs @@ -61,11 +61,21 @@ pub(super) struct Subject<'a> { pub test_path: bool, } +/// The evidence key of the templates a function renders that write values +/// without escaping. +pub(super) const RENDERED: &str = "templates_it_renders_that_write_values_without_escaping"; + impl Subject<'_> { fn code(&self) -> String { format!("{}.source", self.kind) } + /// Whether it renders a template that writes values without escaping, + /// outside Django, whose questions name its templates already. + fn renders(&self) -> bool { + !self.django && self.evidence.contains_key(RENDERED) + } + /// Its name, source and framework evidence, as sent. fn state(&self) -> Value { let mut state = serde_json::Map::new(); @@ -188,6 +198,39 @@ pub(super) fn setup_subject<'a>( }) } +/// A server template's code that reads client data, judged like a function +/// by every security rule. +pub(super) fn template_subject<'a>( + file: &FileContext<'_>, + code: &'a crate::analysis::sites::Setup, +) -> Option> { + let first = code.statements.first()?; + let last = code.statements.last()?; + let source: Vec<&str> = code + .statements + .iter() + .map(|(range, ..)| &file.source[range.clone()]) + .collect(); + Some(Subject { + name: TEMPLATE_CODE.into(), + kind: "function", + source: source.join("\n"), + sites: &code.sites, + errors: &[], + lines: (first.1, last.2), + callers: Vec::new(), + enums: Vec::new(), + constants: Vec::new(), + evidence: serde_json::Map::new(), + django: false, + callee_errors: Vec::new(), + test_path: false, + }) +} + +/// The name of the unit that holds a server template's code. +pub(super) const TEMPLATE_CODE: &str = "template code"; + /// The name of the unit that holds a file's top-level setup statements. pub(super) const MODULE_SETUP: &str = "module setup"; /// The name of that unit in a Django settings module, whose statements @@ -368,11 +411,12 @@ fn presence_request( let source = items[index].1["source"].as_str().unwrap_or_default(); let deserializers = questions::deserializers_named(file.language, source); let xml = questions::parses_xml(file.source, source); + let rendered = !django && items[index].1.get(RENDERED).is_some(); for (rule, _, id) in units { for question in presence_questions(rule) { questions.ask( format!("{}{index}_{question}", &key[..1]), - presence_body(question, &code, django, (deserializers, xml)), + presence_body(question, &code, (django, rendered), (deserializers, xml)), id, rule, question, @@ -404,11 +448,13 @@ pub(super) fn presence_questions(rule: &str) -> &'static [&'static str] { fn presence_body( question: &str, code: &str, - django: bool, + (django, rendered): (bool, bool), (deserializers, xml): (Option<&str>, bool), ) -> Value { match question { - "interpreted" => questions::security_interpreted(code, django, deserializers, xml), + "interpreted" => { + questions::security_interpreted(code, django, rendered, deserializers, xml) + } "resource" => questions::security_resource(code, django), "logs_secret" => questions::security_logs_secret(code), "error_details" => questions::security_error_details(code, django), @@ -579,21 +625,57 @@ fn trace( questions::security_message_origin(&ids, from_callees), ); } - let xml = questions::parses_xml(file.source, &subject.source); - for check in asked_checks(rule, file.language, subject.django, &subject.source, xml) { - let check = if from_callees && check.id == "exception_to_client" { - &questions::EXCEPTION_TO_CLIENT_FROM_CALLEES - } else { - check - }; + for check in trace_checks(file, subject, rule, from_callees) { ask(check.id, check.body(&code)); } + file.request( + "trace", + trace_state(file, subject, rule, messages), + questions, + ) +} + +/// The checks a unit's trace and recheck ask, in the variants its evidence +/// calls for: the exception check of text its callees create, and the +/// markup check of a function that renders unescaped templates. +fn trace_checks( + file: &FileContext<'_>, + subject: &Subject<'_>, + rule: &'static str, + from_callees: bool, +) -> Vec<&'static questions::Check> { + let xml = questions::parses_xml(file.source, &subject.source); + asked_checks(rule, file.language, subject.django, &subject.source, xml) + .into_iter() + // A template's code writes its values unescaped by construction, + // which injection judges; it turns no escaping setting off. Asked + // anyway, a JSP page's `<%= … %>` read as one. + .filter(|check| !(subject.name == TEMPLATE_CODE && check.id == "escape")) + .map(|check| { + if from_callees && check.id == "exception_to_client" { + &questions::EXCEPTION_TO_CLIENT_FROM_CALLEES + } else if subject.renders() && check.id == "markup" { + &questions::VIEW_MARKUP + } else { + check + } + }) + .collect() +} + +/// A trace's state: the unit's source with its sites, and the evidence its +/// rule's checks read. +fn trace_state( + file: &FileContext<'_>, + subject: &Subject<'_>, + rule: &str, + messages: Vec, +) -> Value { let mut state = json!({ "file": file.file_state(), subject.kind: subject.state(), "sites": subject.sites.iter().map(|s| json!({"id": s.id, "source": s.text})).collect::>(), }); - if rule == SENSITIVE_DATA && !messages.is_empty() { state["messages"] = json!(messages); } @@ -606,8 +688,7 @@ fn trace( if rule == UNSAFE_SETTINGS && !subject.constants.is_empty() { state["constants_named"] = json!(subject.constants); } - - file.request("trace", state, questions) + state } /// The origin question and the injection checks again, with the functions @@ -627,14 +708,7 @@ fn recheck(file: &FileContext<'_>, subject: &Subject<'_>, id: &str) -> Option<(V "origin", Pass::Recheck, ); - let xml = questions::parses_xml(file.source, &subject.source); - for check in asked_checks( - INJECTION, - file.language, - subject.django, - &subject.source, - xml, - ) { + for check in trace_checks(file, subject, INJECTION, false) { questions.ask( check.id.into(), check.with_callers(&code), @@ -891,7 +965,9 @@ fn settle( "url_parts" => questions::security_url_parts(&code, callers), "runs_in" => questions::security_runs_in(&code), "redirect_target" => questions::security_redirect_target(&code, callers), - "markup_output" => questions::security_markup_output(&code, subject.django), + "markup_output" => { + questions::security_markup_output(&code, subject.django, subject.renders()) + } "markup_parts" => questions::security_markup_parts(&code, callers), "path_parts" => questions::security_path_parts(&code), "shell_parts" => questions::security_shell_parts(&code), diff --git a/src/units/tests/security.rs b/src/units/tests/security.rs index 4420811..fa921ba 100644 --- a/src/units/tests/security.rs +++ b/src/units/tests/security.rs @@ -443,7 +443,7 @@ fn a_deserializer_is_asked_about_only_where_the_source_names_one() { .replace("pickle.loads", "json.loads"); assert_eq!( first("shop/cart.py", &parsed), - questions::security_interpreted("functions[0].source", false, None, false), + questions::security_interpreted("functions[0].source", false, false, None, false), "code that names no deserializer keeps its question and cached answer" ); assert!(traced_checks("shop/cart.py", PICKLED).contains_key("deserialize")); @@ -1427,3 +1427,113 @@ fn a_function_added_to_one_run_is_the_only_security_request_asked_again() { assert_eq!(sizes, [3, 3, 4, 5]); only_changed(&before, &after, 1); } + +#[test] +fn a_server_template_is_judged_by_its_inline_scripts_only() { + let project = Project::new(); + project.write( + "app/views/sessions/new.html.erb", + "

<%= t('login') %>

\n\n", + ); + project.write( + "app/views/users/show.html.erb", + "

<%= raw @user.bio %>

\n", + ); + let mut options = args(); + options.rules = vec![catalog::INJECTION.into()]; + let (inputs, plan) = planned(&project, &options); + let paths: Vec<_> = inputs.iter().map(|i| i.result.path.clone()).collect(); + assert_eq!( + paths, + [std::path::PathBuf::from("app/views/sessions/new.html.erb")], + "a template without inline scripts is not selected" + ); + let request = &plan.requests[0].request; + assert_eq!(request["state"]["file"]["language"], "JavaScript"); + assert!( + request["state"]["file"]["framework"] + .as_str() + .unwrap() + .contains("runs in the visitor's browser") + ); + let page = &request["state"]["functions"][0]; + assert_eq!(page["name"], "top-level code"); + assert!( + page["source"].as_str().unwrap().contains("document.write("), + "{page}" + ); +} + +#[test] +fn a_node_handler_is_sent_the_unescaped_lines_of_the_view_it_renders() { + let project = Project::new(); + project.write( + "app.js", + "const express = require('express');\nconst app = express();\n\nfunction search(req, res) {\n const term = req.query.q;\n res.render('shop/products', { term });\n}\n\napp.get('/search', search);\n", + ); + project.write( + "views/shop/products.ejs", + "<%- include('../head') %>\n

Results for <%- term %>

\n

<%= term %>

\n", + ); + let mut options = args(); + options.rules = vec![catalog::INJECTION.into()]; + let (_, plan) = planned(&project, &options); + let first = &plan.requests[0].request; + let search = &first["state"]["functions"][0]; + assert_eq!( + search["templates_it_renders_that_write_values_without_escaping"], + json!([{"template": "views/shop/products.ejs", "unescaped_output": ["2:

Results for <%- term %>

"]}]) + ); + let interpreted = first["questions"]["f0_interpreted"]["criteria"]["true"] + .as_str() + .unwrap(); + assert!( + interpreted.contains("passed to a template that writes it unescaped"), + "{interpreted}" + ); + let unit = plan.files[&0] + .units + .iter() + .find(|u| u.rule == catalog::INJECTION) + .unwrap(); + let Detail::Security { + trace: Some(trace), .. + } = &unit.detail + else { + panic!("a traced injection unit"); + }; + let markup = trace.request()["questions"]["markup"]["instructions"]["question"].clone(); + assert!( + markup + .as_str() + .unwrap() + .contains("through a template it renders"), + "{markup}" + ); +} + +#[test] +fn a_template_writing_client_data_unescaped_is_judged_as_template_code() { + let project = Project::new(); + project.write( + "app/views/layouts/application.html.erb", + "\n\n

<%= @title %>

\n\n", + ); + let mut options = args(); + options.rules = vec![catalog::INJECTION.into(), catalog::SENSITIVE_DATA.into()]; + let (inputs, plan) = planned(&project, &options); + assert_eq!(inputs.len(), 1, "selected for its template code alone"); + let names: Vec<(&str, &str)> = plan.files[&0] + .units + .iter() + .map(|u| (u.rule, u.name.as_str())) + .collect(); + assert_eq!( + names, + [(catalog::INJECTION, "template code")], + "an ERB tag is judged for what it writes" + ); + let code = &plan.requests[0].request["state"]["functions"][0]; + assert_eq!(code["source"], "<%= raw cookies[:font] %>"); + assert_eq!(plan.files[&0].units[0].locations[0].start_line, 2); +} From d5b18a53d10114d66164f449b56732908d710060 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 12:57:51 -0300 Subject: [PATCH 2/4] Give the choice of grammar and parsed text its own function --- src/syntax.rs | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/syntax.rs b/src/syntax.rs index c6437b6..41aae6c 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -103,21 +103,30 @@ pub(crate) fn supported(path: &Path) -> bool { || crate::components::server_template(path) } -pub(crate) fn parse(path: &Path, source: &str) -> Result> { +/// The grammar a file is parsed with, and the text parsed in place of its +/// source when that is not its code as written: a component's or server +/// template's scripts, or a project template without its Jinja tags. +fn parsed_text(path: &Path, source: &str) -> Option<(tree_sitter::Language, Option)> { let extension = extension(path); - let server_template = crate::components::server_template(path); - let (language, scripts) = if crate::components::FORMATS.contains(&extension) { + if crate::components::FORMATS.contains(&extension) { let (scripts, language) = crate::components::scripts(extension, source); - (language, Some(scripts)) - } else if server_template { + Some((language, Some(scripts))) + } else if crate::components::server_template(path) { let (scripts, language) = crate::components::scripts("html", source); - (language, Some(without_tags(&scripts, true))) - } else if let Some(language) = grammar(path) { - ( + Some((language, Some(without_tags(&scripts, true)))) + } else { + let language = grammar(path)?; + Some(( language, project_template(path).then(|| without_jinja(source)), - ) - } else { + )) + } +} + +pub(crate) fn parse(path: &Path, source: &str) -> Result> { + let extension = extension(path); + let server_template = crate::components::server_template(path); + let Some((language, scripts)) = parsed_text(path, source) else { return Ok(None); }; // A template's tree is of its code without the Jinja tags, apart from From 0688168ce31b45324012d0499f5a7f30602f7dc7 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Sat, 26 Sep 2026 13:03:06 -0300 Subject: [PATCH 3/4] Tell Pug's unescaped output from a comparison, and name templates in check's help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pug writes a value unescaped with `!=` right after a tag or at the start of a line, and with `!{…}`; `err.name !== 'AbortError'` in a view's script is a comparison. hackathon-starter's handlers were sent such comparisons as unescaped output. `check --help` names the component and server template files JevGate selects. --- src/analysis/views.rs | 23 +++++++++++++++++++++++ src/options/mod.rs | 8 +++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/analysis/views.rs b/src/analysis/views.rs index fe4d222..106182b 100644 --- a/src/analysis/views.rs +++ b/src/analysis/views.rs @@ -45,10 +45,14 @@ pub fn view(relative: &Path, text: &str) -> Option