White-Box Testing

Intermediate

A05 | Injection A03 | Software Supply Chain Failures

JavaScript and Node.js Security

JavaScript is the only language on this site where the review has to happen twice, in two different trust models: a browser you do not control, and a server that runs every dependency that was installed for it. The defects are the same shapes - injection, deserialisation by another name, path handling - but the sinks are spread across DOM APIs and a package manager.

Two runtimes, one review

Client-side sinks are reachable by anyone who can get a value rendered. Server-side sinks are reachable by anyone who can reach the route. Review both, and do not accept "it is escaped in the frontend" as a finding closure for a server-side defect - the API is callable without the frontend.

Client-side sinks

A DOM XSS finding is a statement about a sink, not about an input. The same comment object is safe in one component and exploitable in the next, because one renders it as text and the other parses it as markup. Reviewers who audit the fetch layer and stop there report either nothing or everything.

The sinks that parse markup are short and stable: innerHTML, outerHTML, insertAdjacentHTML, document.write, and the framework wrappers around them. Everything else writes text, attributes or properties, and none of those execute.

comment-view.js

// the fetch layer is careful; the render layer is not
const res = await fetch('/api/comment/' + id);
const comment = await res.json();

document.getElementById('body').innerHTML = comment.body;
preview.insertAdjacentHTML('beforeend', comment.author);
document.write('<p>' + comment.body + '</p>');

// React equivalent, same defect
return <div dangerouslySetInnerHTML={{ __html: comment.body }} />;

// the fetch layer is careful; the render layer is not
const res = await fetch('/api/comment/' + id);
const comment = await res.json();

document.getElementById('body').innerHTML = comment.body;
preview.insertAdjacentHTML('beforeend', comment.author);
document.write('<p>' + comment.body + '</p>');

// React equivalent, same defect
return <div dangerouslySetInnerHTML={{ __html: comment.body }} />;

import DOMPurify from 'dompurify';

// text only: no parsing, no execution
document.getElementById('body').textContent = comment.body;
preview.textContent = comment.author;

// markup genuinely needed: sanitise at the sink, allowlist in place
const clean = DOMPurify.sanitize(comment.body, { USE_PROFILES: { html: true } });
document.getElementById('body').innerHTML = clean;

// React: render as data unless rich text is a requirement
return <div>{comment.body}</div>;

import DOMPurify from 'dompurify';

// text only: no parsing, no execution
document.getElementById('body').textContent = comment.body;
preview.textContent = comment.author;

// markup genuinely needed: sanitise at the sink, allowlist in place
const clean = DOMPurify.sanitize(comment.body, { USE_PROFILES: { html: true } });
document.getElementById('body').innerHTML = clean;

// React: render as data unless rich text is a requirement
return <div>{comment.body}</div>;

Trusted Types makes the finding permanent

On a browser that supports it, a Trusted Types policy turns every string-to-markup sink into a runtime error unless a policy produced the value. That is the difference between a fix that survives the next developer and one that survives the next sprint.

eval, Function and the vm module

The eval family is small and most of it is banned by lint rules, so what remains in a real codebase is the deliberate case: a rules engine, a plugin loader, a formula field, or a template compiler. Those are findings when the expression comes from a request, and they are still findings when it comes from a database row that a user can write.

rules.js

// "configurable rules", compiled at request time
const rule = new Function('amount', req.body.expression);
const total = rule(100);

// worse: a plugin system that evaluates anything it is handed
eval(req.query.hook);

// setTimeout with a string evaluates it, exactly like eval
setTimeout(req.body.callback, 0);

// the vm module shares the process: this is not a sandbox
const vm = require('node:vm');
vm.runInNewContext(req.body.script, { console });

// "configurable rules", compiled at request time
const rule = new Function('amount', req.body.expression);
const total = rule(100);

// worse: a plugin system that evaluates anything it is handed
eval(req.query.hook);

// setTimeout with a string evaluates it, exactly like eval
setTimeout(req.body.callback, 0);

// the vm module shares the process: this is not a sandbox
const vm = require('node:vm');
vm.runInNewContext(req.body.script, { console });

// expressions are data, parsed against a grammar you own
const allowed = new Map([
  ['subtotal', (n) => n],
  ['with-tax', (n) => n * 1.2],
]);

const fn = allowed.get(req.body.expression);
if (!fn) return res.status(400).json({ error: 'unknown expression' });
const total = fn(amount);

// scheduled work takes a function, never a string
setTimeout(() => runQueuedTask(req.body.taskId), 0);

// untrusted code needs a process boundary, not a context
// spawn a worker in a separate process with its own uid and fs access

// expressions are data, parsed against a grammar you own
const allowed = new Map([
  ['subtotal', (n) => n],
  ['with-tax', (n) => n * 1.2],
]);

const fn = allowed.get(req.body.expression);
if (!fn) return res.status(400).json({ error: 'unknown expression' });
const total = fn(amount);

// scheduled work takes a function, never a string
setTimeout(() => runQueuedTask(req.body.taskId), 0);

// untrusted code needs a process boundary, not a context
// spawn a worker in a separate process with its own uid and fs access

The vm module is not a sandbox

node:vm contexts share the process, the filesystem and the network with the host. Node's own documentation says so, and every escape from a vm context that has ever been published relies on that. Untrusted code needs a separate process with a separate identity, or it needs to not run.

Why setTimeout with a string is the same bug

The first argument of setTimeout and setInterval is evaluated when it is a string, which is the same parsing step that makes eval dangerous. It is rare in new code and common in code that was ported from a pre-promise callback style, which is exactly the code nobody re-reviews. Grep for it explicitly rather than relying on the eval rule.

Prototype pollution

Prototype pollution (CWE-1321) is not an injection into a value; it is an injection into the object graph. A recursive merge that copies keys from a parsed JSON body lets an attacker write __proto__ and add a property that every object in the process will then inherit. The finding is the merge function, and the impact depends on which property is read later - an authorisation flag is the version that matters.

merge.js

// merge used for config, request bodies and query strings alike
function merge(target, source) {
  for (const key in source) {
    if (typeof source[key] === 'object' && source[key] !== null) {
      target[key] = merge(target[key] || {}, source[key]);
    } else {
      target[key] = source[key];
    }
  }
  return target;
}

// {"__proto__": {"isAdmin": true}} on any JSON body
merge(defaults, req.body);

// every object in the process now answers this
if (user.isAdmin) { /* never set explicitly */ }

// merge used for config, request bodies and query strings alike
function merge(target, source) {
  for (const key in source) {
    if (typeof source[key] === 'object' && source[key] !== null) {
      target[key] = merge(target[key] || {}, source[key]);
    } else {
      target[key] = source[key];
    }
  }
  return target;
}

// {"__proto__": {"isAdmin": true}} on any JSON body
merge(defaults, req.body);

// every object in the process now answers this
if (user.isAdmin) { /* never set explicitly */ }

function merge(target, source) {
  for (const key of Object.keys(source)) {
    // own keys only, and never a prototype path
    if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue;

    const value = source[key];
    if (value && typeof value === 'object' && !Array.isArray(value)) {
      // null prototype: no inherited setters to poison
      target[key] = merge(Object.hasOwn(target, key) ? target[key] : Object.create(null), value);
    } else {
      target[key] = value;
    }
  }
  return target;
}

const config = merge(Object.create(null), req.body);
// authorisation then reads config.isAdmin, which the attacker cannot reach

function merge(target, source) {
  for (const key of Object.keys(source)) {
    // own keys only, and never a prototype path
    if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue;

    const value = source[key];
    if (value && typeof value === 'object' && !Array.isArray(value)) {
      // null prototype: no inherited setters to poison
      target[key] = merge(Object.hasOwn(target, key) ? target[key] : Object.create(null), value);
    } else {
      target[key] = value;
    }
  }
  return target;
}

const config = merge(Object.create(null), req.body);
// authorisation then reads config.isAdmin, which the attacker cannot reach

Where it hides	What to check	Fixes that hold
Config merge	Does the merge recurse over keys from a request or a file?	Null-prototype target, deny prototype keys
Query parsing	Nested query syntax such as a[b]=c, and any deep parser	Disable nested parsing, or parse into a Map
Clone helpers	Hand-written deep clone that assigns by key	structuredClone, or an allowlist of keys
Third-party merge	Old lodash-style merge accepting a key path	Patch, and check call sites for reachability

Command execution in Node

Node gives two different primitives and names them clearly enough that the defect is visible in the import: exec hands a string to a shell, and execFile and spawn do not. The defect that survives review is the one where the import is correct and shell: true was added later to make a pipeline work.

convert.js

const { exec } = require('node:child_process');

// the handler builds a command string out of request data
app.post('/convert', (req, res) => {
  exec('convert ' + req.body.file + ' /tmp/out.png', (err, stdout) => {
    res.send(stdout);
  });
});

// shell: true re-introduces interpretation even with spawn
spawn('convert ' + req.body.file + ' /tmp/out.png', { shell: true });

const { exec } = require('node:child_process');

// the handler builds a command string out of request data
app.post('/convert', (req, res) => {
  exec('convert ' + req.body.file + ' /tmp/out.png', (err, stdout) => {
    res.send(stdout);
  });
});

// shell: true re-introduces interpretation even with spawn
spawn('convert ' + req.body.file + ' /tmp/out.png', { shell: true });

const { execFile } = require('node:child_process');

app.post('/convert', (req, res) => {
  // argument array, no shell, command fixed in code
  execFile('convert', [req.body.file, '/tmp/out.png'], (err, stdout) => {
    if (err) return res.status(500).json({ error: 'conversion failed' });
    res.send(stdout);
  });
});

// authorisation belongs on the file argument, not on the command
const resolved = path.resolve(UPLOAD_DIR, req.body.file);
if (!resolved.startsWith(UPLOAD_DIR + path.sep)) return res.sendStatus(400);

const { execFile } = require('node:child_process');

app.post('/convert', (req, res) => {
  // argument array, no shell, command fixed in code
  execFile('convert', [req.body.file, '/tmp/out.png'], (err, stdout) => {
    if (err) return res.status(500).json({ error: 'conversion failed' });
    res.send(stdout);
  });
});

// authorisation belongs on the file argument, not on the command
const resolved = path.resolve(UPLOAD_DIR, req.body.file);
if (!resolved.startsWith(UPLOAD_DIR + path.sep)) return res.sendStatus(400);

Sanitising the argument is not the fix

Escaping shell metacharacters is a denylist against a shell you did not write. Removing the shell is the fix. After that, the remaining question is whether the binary itself interprets its arguments - a file path passed to an image converter is still an argument, but a path passed to a shell wrapper script is a command again.

Express and templating

Express has no output encoding because it has no output model: it sends what you give it and infers the content type from the value. A string becomes text/html, so res.send with a concatenated string is a DOM sink on the server. EJS and Handlebars both escape by default and both ship an unescaped variant that is used far more often than the docs suggest.

routes.js

// EJS: <%= escapes, <%- does not. Both compile to the same page.
<p>Hello <%- user.displayName %></p>

// res.send with a string sets text/html, so the browser parses it
app.get('/search', (req, res) => {
  res.send('<h1>Results for ' + req.query.q + '</h1>');
});

// template literal into a markup response: same sink, different syntax
res.send(`<div class="notice">${req.query.message}</div>`);

// EJS: <%= escapes, <%- does not. Both compile to the same page.
<p>Hello <%- user.displayName %></p>

// res.send with a string sets text/html, so the browser parses it
app.get('/search', (req, res) => {
  res.send('<h1>Results for ' + req.query.q + '</h1>');
});

// template literal into a markup response: same sink, different syntax
res.send(`<div class="notice">${req.query.message}</div>`);

// EJS: escape by default, and pass data rather than markup
<p>Hello <%= user.displayName %></p>

// JSON endpoint: data, not markup, and a type the browser will not parse
app.get('/search', (req, res) => {
  res.type('application/json').send({ query: req.query.q, results });
});

// if HTML must be returned, build it from escaped values
res.send('<div class="notice">' + escapeHtml(req.query.message) + '</div>');

// helmet sets the headers that make an injected script fail to run
app.use(helmet());

// EJS: escape by default, and pass data rather than markup
<p>Hello <%= user.displayName %></p>

// JSON endpoint: data, not markup, and a type the browser will not parse
app.get('/search', (req, res) => {
  res.type('application/json').send({ query: req.query.q, results });
});

// if HTML must be returned, build it from escaped values
res.send('<div class="notice">' + escapeHtml(req.query.message) + '</div>');

// helmet sets the headers that make an injected script fail to run
app.use(helmet());

Mass assignment is the same class of bug in a different API

Object.assign(user, req.body) and ORM equivalents copy whichever fields the client sent, including role and isAdmin. The review question is not whether the body is validated, but whether the write path enumerates the fields it accepts. A schema that allowlists fields answers both this and the prototype-pollution question at once.

Path handling

Three APIs, three different behaviours, and only one of them answers the question the code is trying to ask. path.join normalises traversal segments rather than rejecting them; path.basename throws away the directory, so it prevents traversal by silently serving a different file than the one requested; and path.resolve with a prefix test is the only one that expresses containment.

download.js

const path = require('node:path');

// join normalises, so ../ walks out of the directory
const file = path.join(__dirname, 'uploads', req.params.name);
fs.readFileSync(file);

// basename strips the directory - it is not a containment check
const safeName = path.basename(req.params.name);
fs.readFileSync(path.join(UPLOAD_DIR, safeName));

// resolve without a boundary check still permits /uploads/../../etc/passwd
const target = path.resolve('uploads', req.params.name);
fs.readFileSync(target);

const path = require('node:path');

// join normalises, so ../ walks out of the directory
const file = path.join(__dirname, 'uploads', req.params.name);
fs.readFileSync(file);

// basename strips the directory - it is not a containment check
const safeName = path.basename(req.params.name);
fs.readFileSync(path.join(UPLOAD_DIR, safeName));

// resolve without a boundary check still permits /uploads/../../etc/passwd
const target = path.resolve('uploads', req.params.name);
fs.readFileSync(target);

const path = require('node:path');

const target = path.resolve(UPLOAD_DIR, req.params.name);

// containment is a prefix test on the resolved path, with a separator
if (!target.startsWith(UPLOAD_DIR + path.sep)) {
  return res.sendStatus(403);
}

// and a symlink inside the directory can still point outside it
const real = fs.realpathSync(target);
if (!real.startsWith(UPLOAD_DIR + path.sep)) return res.sendStatus(403);

fs.readFileSync(real);

const path = require('node:path');

const target = path.resolve(UPLOAD_DIR, req.params.name);

// containment is a prefix test on the resolved path, with a separator
if (!target.startsWith(UPLOAD_DIR + path.sep)) {
  return res.sendStatus(403);
}

// and a symlink inside the directory can still point outside it
const real = fs.realpathSync(target);
if (!real.startsWith(UPLOAD_DIR + path.sep)) return res.sendStatus(403);

fs.readFileSync(real);

Realpath, because the filesystem is the second input

A prefix test on the resolved path is defeated by a symlink inside the upload directory, and by a case-insensitive filesystem on Windows or macOS. Resolve the real path after the prefix test, and compare again. The fix is boring, which is why it is skipped.

The install path is attack surface

The 2025 OWASP Top 10 moved supply chain to A03 for a reason that is easy to see in a JavaScript review: a dependency's install script runs with the same privileges as the build, before any of your code executes. npm install runs lifecycle scripts by default; npm ci installs exactly the lockfile and still runs them.

Control	What it actually prevents	What it does not
Lockfile committed	Version drift between machines and CI	A malicious version published under a range
npm ci	Installing something other than the lockfile	Lifecycle scripts, which still run
ignore-scripts=true	postinstall execution during install and CI	Malicious code in the package's runtime entry point
Audit and SCA in CI	Known advisories against resolved versions	A fresh takeover with no advisory yet

Four lines that remove most of the exposure

Commit the lockfile, install with npm ci, set ignore-scripts=true in the project .npmrc except where a native dependency genuinely needs to build, and run an SCA scanner on every pull request. The exception is what a reviewer should be looking for - a repository that disables scripts globally and then re-enables them for one package has documented its own weakest link.

Check	Expected	Where to look
Every DOM write uses a sink that does not parse markup	textContent, value, or a sanitiser such as DOMPurify immediately at the sink — never innerHTML with interpolated data	The render layer, not the fetch layer
No eval-family call receives data that crosses a trust boundary	No eval, new Function, setTimeout with a string, or template compilation of user-supplied source	Grep for the constructors, then trace each argument back to an input
Recursive merge and clone functions reject prototype keys	A denylist for __proto__, constructor and prototype, or a null-prototype target plus an own-property check	The utility that merges config, body or query objects
Process execution passes an argument array and never a shell string	execFile, spawn or execFileSync with shell disabled, and the command itself fixed rather than concatenated	The operation wrapper, one layer above the handler

Evidence Collection

A JavaScript finding is only convincing with the sink in the quote. Record these before leaving the file:

The sink and its line number: innerHTML, eval, exec, merge - the construct itself, not the variable name. The variable is named "safe" in half of these.

Where the data enters: Route, parameter, or a third-party response. If it is a dependency response, say which one and which field - that is what decides whether the fix is local.

Whether the value is stored or reflected: A stored sink runs for every reader, which changes severity and the remediation order.

Bedrock Questions Before Filing

JavaScript produces more false positives than any other language here, mostly because frameworks escape in places the reviewer is not looking:

Is the value actually reaching a parser: textContent, React children and Vue text interpolation do not parse markup. A value passed through them is not an XSS finding, however untrusted it is.
Is the string a constant: eval of a literal, or a template compiled from a file on disk, is a code-quality issue. The finding is the path from an input to that construct.
Is the merge reachable from a request: Prototype pollution needs an attacker-controlled key. A merge over two internal config files is not the same finding as a merge over req.body.
Does the shell actually exist: execFile and spawn without shell do not interpret metacharacters. Half of the command-injection reports against Node are against these.

Reviewer Checklist

Five minutes per file, in this order. Nothing here needs a scanner.

☐ Grep for innerHTML, outerHTML, insertAdjacentHTML, document.write, dangerouslySetInnerHTML
☐ Confirm every remaining sink is fed a sanitiser output or a constant
☐ Check that sanitisation happens at the sink, not at the fetch
☐ Look for postMessage handlers that accept a message from any origin

☐ Grep for eval, new Function, setTimeout with a string, and the vm module
☐ Trace each argument back to a request, a database field or a file
☐ Reject "we only pass trusted expressions" without seeing where the trusted set is enforced

☐ Grep for child_process and check every call for a shell string
☐ Check path handling for a resolve plus prefix test, not basename
☐ Check EJS and Handlebars templates for unescaped interpolation
☐ Check that res.send never receives a concatenated string with request data

☐ Confirm a lockfile is committed and the install command is ci, not install
☐ Confirm lifecycle scripts are disabled where the pipeline does not need them
☐ Read the audit output once per release, and triage by reachability rather than by count

Remediation Notes

Fixes that hold are structural. Fixes that regress are string replacements.

What the framework already escapes, and what it does not

React escapes children and attribute values, so JSX interpolation is not a sink. It does not escape href with a javascript: scheme, and it does not touch dangerouslySetInnerHTML. Vue escapes mustaches and not v-html. Angular sanitises by default and not when bypassSecurityTrust is called. In each case the framework has one documented escape hatch, and that hatch is where the finding lives.

JavaScript Review Practice Targets

Applications with source you can read, where the sinks are real and the intended answer is not in a hint box.

OWASP Juice Shop Custom Lab medium

Audit the Express routes for client-side and server-side sinksFind the DOM XSS challenges in the Angular frontendTrace a stored value from the API to a render sink

OWASP NodeGoat Custom Lab easy

Deliberately vulnerable Express handlersCommand execution and NoSQL injection samplesSmall enough to review end to end

Damn Vulnerable NodeJS Application Custom Lab easy

Node-specific sink catalogueContainer-based setupContrast the scanner output with a manual pass

Need help setting up? Check our Lab Setup Guide →

SAST Fundamentals

Taint analysis and the sink catalogue these findings come out of.

Manual Code Review

The review pass that puts these sinks in context.

Cross-Site Scripting

The fix catalogue for the output-encoding defects found here.

Injection

Root causes and parameterised fixes across languages.

CI/CD Supply Chain

Where the install-time findings turn into pipeline compromise.