Skip to content

Add vulnerability lab API for security testing (BOLA, RCE, LLM injection, SSRF, SSTI, and more) - #7

Open
Bonckheere1 wants to merge 1 commit into
masterfrom
claude/vulnerable-code-security-testing-p1om9x
Open

Bonckheere1 wants to merge 1 commit into
masterfrom
claude/vulnerable-code-security-testing-p1om9x

Conversation

@Bonckheere1

Copy link
Copy Markdown
Owner

Summary

This fork of OWASP NodeGoat already exercises several classic vulnerability classes (IDOR in allocations.js, SQL injection in reports.js, NoSQL $where injection in allocations-dao.js, broken auth/session handling, XSS/CSRF/open redirect, and a basic SSRF in research.js). This PR adds a dedicated /api vulnerability lab to round out the full set of categories needed for security testing, plus a couple of additional IDOR/business-logic examples:

  • Broken Access Control (BOLA/IDOR)GET /api/orders/:orderId, GET /api/users/:userId/profile
  • Business Logic & ValidationPOST /api/coupon/redeem, POST /api/wallet/transfer
  • Code & Command InjectionPOST /api/tools/ping (child_process.exec with unsanitized input)
  • SQL & Database InjectionPOST /api/users/authenticate (MongoDB operator injection / auth bypass)
  • LLM & Prompt InjectionPOST /api/assistant/chat (untrusted input concatenated into a privileged system prompt)
  • SSRFPOST /api/tools/fetch-url (no allow-list, reaches internal/metadata endpoints)
  • Authentication & Session Management — hand-rolled JWT (app/lib/jwt-lab.js) that accepts "alg":"none" and skips signature verification
  • Client-Side AttacksGET /api/echo (reflected XSS), GET /api/proxy-info (web cache poisoning via unkeyed X-Forwarded-Host)
  • Insecure Deserialization & SSTIPOST /api/settings/import (eval()-based deserialization), POST /api/notify/render (user-controlled Swig template)
  • Files & MisconfigurationsPOST /api/files/upload (unrestricted upload), GET /api/files/read (LFI/path traversal), GET /api/files/list (directory listing), plus full stack traces now rendered to every client in app/routes/error.js
  • Secrets & Cryptography — hardcoded API key/JWT secret in app/data/lab-store.js, MD5-based predictable password-reset tokens returned in the API response, an admin bypass gated only by a hardcoded API key
  • Hardening — permissive reflected-origin CORS middleware on /api/*, and a minimal GraphQL endpoint (app/lib/graphql-lab.js) with introspection always enabled, no query depth limiting, and no field-level authorization

Every endpoint has an inline comment explaining the exact bug and its fix. Full endpoint-by-endpoint documentation with example requests (curl commands, payloads) is in VULNERABILITY_LABS.md, including a table of the pre-existing vulnerabilities already in this fork for context.

All new code relies only on dependencies already declared in package.json (mongodb, needle, swig, plus Node built-ins) — no new npm packages were added.

This code is intentionally insecure and is for authorized security testing / training in an isolated environment only. Do not deploy it anywhere internet-reachable.

Test plan

  • node --check passes on all new/modified files (no node_modules available in the sandbox this was built in, so a full npm install && npm start run plus manual exercise of each endpoint from VULNERABILITY_LABS.md is recommended before use)
  • Run npm install && npm run db:seed && npm start, then exercise each endpoint listed in VULNERABILITY_LABS.md against a scanner/manual testing tool
  • Confirm existing NodeGoat routes/tests are unaffected (only additive changes to app/routes/index.js, app/routes/error.js, and app/views/error-template.html)

Generated by Claude Code

…eserialization/SSTI, file/misconfig, secrets/JWT, and GraphQL/CORS hardening gaps

This fork already exercises IDOR, SQLi, NoSQL $where injection, broken auth,
XSS/CSRF/open redirect, and SSRF in the existing NodeGoat routes. This adds a
dedicated /api lab (app/routes/api.js, app/lib/jwt-lab.js,
app/lib/graphql-lab.js, app/data/lab-store.js) plus a couple of additional
BOLA/IDOR and business-logic examples, to round out the full set of
categories requested for security testing. Every endpoint is commented with
the exact bug and its fix; see VULNERABILITY_LABS.md for a full writeup with
example requests. All new code relies only on dependencies already declared
in package.json (no new npm packages).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JrBAz3oy7qW1tesyLMU2CV
Comment thread app/routes/api.js
try {
const decoded = Buffer.from(req.body.data || "", "base64").toString("utf8");
/*jslint evil: true */
const settings = eval(`(${decoded})`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remote Code Execution possible via eval()-type functions - critical severity
Using functions such as eval, but also less obvious functions such as setTimeout, setInterval or 'new Function' can lead to users being able to run their own code on your servers.

Show fix

Remediation: If possible, avoid using these functions altogether. If not, use a list of allowed inputs that can feed into these functions.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// together with strict input validation (e.g. a hostname/IP regex).
router.post("/tools/ping", isLoggedIn, (req, res) => {
const host = req.body.host || "";
exec(`ping -c 1 ${host}`, { timeout: 5000 }, (err, stdout, stderr) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential for OS command injection via child_process call - critical severity
It is generally not recommended to call out to the operating system to execute commands. When the application is executing file-system-based commands, user input should never be used in constructing commands or command arguments.

Show fix

Remediation: If you can, avoid the exec() call. If that is not possible, consider hard coding both the command and arguments to be used. Alternatively, install Aikido Runtime for NodeJS to prevent command injection completely.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// Fix: reject non-string userName/password before querying, and prefer an
// explicit equality comparison after validating types.
router.post("/users/authenticate", (req, res, next) => {
usersCol.findOne({ userName: req.body.userName, password: req.body.password }, (err, user) => {

@aikido-pr-checks aikido-pr-checks Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NoSQL injection attack possible - critical severity
Query injection attacks are possible if users can pass objects instead of strings to query functions such as findOne.

By injecting query operators attackers can control the behavior of the query, allowing them to bypass access controls and extract unauthorized data. Consider the attack payload ?user_id[$ne]=5: if the user_id query parameter is passed to the query function without validation or casting its type, an attacker can pass {$ne: 5} instead of an integer to the query. {$ne: 5} uses the 'not equal to' operator to access data of other users.

While this vulnerability is known as NoSQL injection, relational databases (mysql, postgres) are also vulnerable to this attack if the query library offers a NoSQL-like API and supports string-typed query operators. Examples include prisma and sequelize versions prior to 4.12.0.

Suggested change
usersCol.findOne({ userName: req.body.userName, password: req.body.password }, (err, user) => {
usersCol.findOne({ userName: { $eq: req.body.userName }, password: { $eq: req.body.password } }, (err, user) => {

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// the account holder - anyone who knows a username can self-serve a valid
// reset token for that account.
router.post("/auth/forgot-password", (req, res, next) => {
usersCol.findOne({ userName: req.body.userName }, (err, user) => {

@aikido-pr-checks aikido-pr-checks Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NoSQL injection attack possible - critical severity
Query injection attacks are possible if users can pass objects instead of strings to query functions such as findOne.

By injecting query operators attackers can control the behavior of the query, allowing them to bypass access controls and extract unauthorized data. Consider the attack payload ?user_id[$ne]=5: if the user_id query parameter is passed to the query function without validation or casting its type, an attacker can pass {$ne: 5} instead of an integer to the query. {$ne: 5} uses the 'not equal to' operator to access data of other users.

While this vulnerability is known as NoSQL injection, relational databases (mysql, postgres) are also vulnerable to this attack if the query library offers a NoSQL-like API and supports string-typed query operators. Examples include prisma and sequelize versions prior to 4.12.0.

Suggested change
usersCol.findOne({ userName: req.body.userName }, (err, user) => {
usersCol.findOne({ userName: { $eq: req.body.userName } }, (err, user) => {

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// passed as *context*, never as the template itself.
router.post("/notify/render", isLoggedIn, (req, res) => {
try {
const html = swig.render(req.body.template || "", { locals: { name: req.body.name || "" } });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server-Side Template Injection via untrusted input in express.render() - critical severity
Direct use of user-controlled inputs as arguments to the express.render() function can result in server-side template injection when the template engine evaluates untrusted data. An attacker may craft malicious payloads to read local files or, depending on the template engine and its configuration, escalate the issue to remote code execution by abusing template logic and expression handling.

Show fix

Remediation: Validate and sanitize all inputs before rendering, never pass raw user objects to templates, restrict template capabilities, and enforce strict variable whitelisting or safe rendering modes.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// existing file outside the intended uploads directory.
router.post("/files/upload", isLoggedIn, (req, res) => {
const { filename, contentBase64 } = req.body;
const destination = path.join(uploadsDir, filename || "");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.

Show fix

Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// Inclusion / path traversal read primitive, e.g.
// /api/files/read?path=../../../../../../etc/passwd
router.get("/files/read", isLoggedIn, (req, res) => {
const target = path.join(uploadsDir, req.query.path || "");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.

Show fix

Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@aikido-pr-checks

aikido-pr-checks Bot commented Aug 11, 2026

Copy link
Copy Markdown

Summary by Aikido

⚠️ Security Issues: 11 Quality Issues: 0 ✅ Resolved Issues: 1

🚀 New Features

  • Added dedicated /api vulnerability lab covering injection, access-control, SSRF, and file flaws
  • Implemented JWT and GraphQL helpers for authentication and authorization security testing

⚡ Enhancements

  • Exposed full error stack traces through the global error response

📚 Documentation

  • Documented every vulnerability-lab endpoint with examples, risks, and remediation guidance

More info

Comment thread app/routes/api.js
// /api/files/read?path=../../../../../../etc/passwd
router.get("/files/read", isLoggedIn, (req, res) => {
const target = path.join(uploadsDir, req.query.path || "");
fs.readFile(target, "utf8", (err, contents) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.

Show fix

Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// Directory listing + traversal: returns raw directory entries for any
// path reachable via "..", with no restriction back to uploadsDir.
router.get("/files/list", isLoggedIn, (req, res) => {
const target = path.join(uploadsDir, req.query.dir || ".");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.

Show fix

Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/api.js
// and only allow a small allow-list of external hosts.
router.post("/tools/fetch-url", isLoggedIn, (req, res) => {
const url = req.body.url || "";
needle.get(url, { follow_max: 3 }, (err, response) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP request might enable SSRF attack - high severity
If an attacker can control the URL input leading into this http request, the attack might be able to perform an SSRF attack. This kind of attack is even more dangerous is the application returns the result of the URL fetch to the user. It can serve as an initial access point for an attacker for stealing credentials in the cloud.

Show fix

Remediation: If possible, only allow requests to verified domains. If not, consult the article linked above to learn about other mitigating techniques such as disabling redirects, blocking private IPs and making sure private services have internal authentication. If you return data coming from the request to the user, validate the data before returning it to make sure you don't return random data.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread app/routes/error.js
Comment on lines 20 to 23
res.render("error-template", {
error: err
error: err,
stack: err.stack
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server-Side Template Injection via untrusted input in express.render() - medium severity
Direct use of user-controlled inputs as arguments to the express.render() function can result in server-side template injection when the template engine evaluates untrusted data. An attacker may craft malicious payloads to read local files or, depending on the template engine and its configuration, escalate the issue to remote code execution by abusing template logic and expression handling.

Show fix

Remediation: Validate and sanitize all inputs before rendering, never pass raw user objects to templates, restrict template capabilities, and enforce strict variable whitelisting or safe rendering modes.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 28 critical · 29 high

Alerts:
⚠ 57 issues (≤ 0 issues of at least minor severity)

Results:
57 new issues

Category Results
ErrorProne 6 high
Security 28 critical
23 high

View in Codacy

🟢 Metrics 104 complexity · 0 duplication

Metric Results
Complexity 104
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copy link
Copy Markdown
Owner Author

Deployability check

Confirmed this diff is a real, integrated addition to the app rather than standalone/dead code:

  • Wiring: app/routes/api.js is required and mounted exactly like the existing tutorialRouter (app.use("/api", apiRouter(db, isLoggedIn)) in app/routes/index.js), reusing the same db connection and isLoggedIn middleware the rest of the app uses — no parallel app instance, no new boot path.
  • Dependencies: zero new npm packages. Everything runs on what's already in package.json (mongodb, needle, swig) plus Node built-ins (crypto, fs, path, child_process). The Dockerfile's npm install --production and the existing CI config need no changes.
  • Behavioral verification: this sandbox's egress policy blocks registry.npmjs.org (x-deny-reason: host_not_allowed), so a real npm install && npm start wasn't possible here. Instead I wrote a throwaway test harness (not part of this PR) that required the real app/routes/api.js module against a fake Express Router/Mongo-shaped db/needle/swig, invoked all 21 new routes through their full middleware chain (CORS → isLoggedIn → handler) with realistic payloads, and asserted both the "normal" and "exploit" behavior for each. Result: 60/60 assertions passed (4 initial failures were bugs in the test harness itself — missing session.userId on a couple of authenticated calls, an off-by-one in a path-traversal depth — not in the app code; fixed the harness and re-ran clean). This exercised, for example: IDOR reads across users, the negative-price coupon flaw, the NoSQL $ne auth bypass, the prompt-injection secret leak, the SSRF fetch, the JWT alg:none forgery, reflected XSS, eval()-based deserialization, SSTI expression evaluation, upload → path-traversal read-back, the MD5 reset-token flow, the hardcoded-API-key admin bypass, and GraphQL introspection/BOLA.
  • Lint: all new files pass node --check, and manually verified against this repo's .jshintrc (double-quote strings, UPPER_CASE constants allowed by the camelcase rule).

Recommend still doing one real npm install && npm start + manual pass per VULNERABILITY_LABS.md before relying on this for testing, since the harness above substitutes lightweight stand-ins for express/mongodb/needle/swig rather than the real libraries.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants