diff --git a/CHANGELOG.md b/CHANGELOG.md
index 208184e..ebd2ab7 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/options/mod.rs b/src/options/mod.rs
index e9b6b60..1f443d8 100644
--- a/src/options/mod.rs
+++ b/src/options/mod.rs
@@ -100,9 +100,11 @@ pub struct CheckArgs {
///
/// Without paths, JevGate walks the repository (respecting .gitignore) and
/// selects application source in Rust, Python, JavaScript, TypeScript, Go,
- /// C#, Ruby, PHP and Java. Tests, generated code and vendored files are
- /// classified and skipped with a reason. `upload_allow`/`upload_deny` in
- /// jevgate.toml still bound what is sent.
+ /// C#, Ruby, PHP and Java, the scripts of Astro, Vue and Svelte files, and
+ /// server templates (ERB, EJS, JSP, Handlebars, Jinja and others) that
+ /// hold inline scripts or code reading the request. Tests, generated code
+ /// and vendored files are classified and skipped with a reason.
+ /// `upload_allow`/`upload_deny` in jevgate.toml still bound what is sent.
pub paths: Vec,
/// Review only files changed against this Git revision (commit, branch or tag)
///
diff --git a/src/syntax.rs b/src/syntax.rs
index ce03bb6..41aae6c 100644
--- a/src/syntax.rs
+++ b/src/syntax.rs
@@ -98,20 +98,35 @@ 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> {
+/// 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 (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 let Some(language) = grammar(path) {
- (
+ Some((language, Some(scripts)))
+ } else if crate::components::server_template(path) {
+ let (scripts, language) = crate::components::scripts("html", source);
+ 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
@@ -136,7 +151,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 +193,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 +226,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 +330,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') %>\nResults 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);
+}