Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions app/data/reports-dao.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
* A1 - SQL Injection
*
* This module demonstrates SQL Injection vulnerabilities in a SQLite-backed
* payroll reports feature. User-supplied input is concatenated directly into
* SQL query strings instead of using parameterized queries.
*
* Attack examples:
* Search name: ' OR '1'='1 -> dumps all employee records
* Search name: ' OR 1=1-- -> bypasses filtering
* Search name: '; DROP TABLE employees;-- -> destructive injection
* Search name: ' UNION SELECT id,username,password,salary,0 FROM users-- -> data exfil
*/

const sqlite3 = require("sqlite3").verbose();
const path = require("path");

// Use an in-memory database pre-seeded with sample payroll data
let dbInstance = null;

function getDb() {
if (dbInstance) return dbInstance;

dbInstance = new sqlite3.Database(":memory:");

dbInstance.serialize(() => {
// Create employees table with sensitive payroll data
dbInstance.run(`
CREATE TABLE IF NOT EXISTS employees (
id INTEGER PRIMARY KEY AUTOINCREMENT,
name TEXT NOT NULL,
department TEXT NOT NULL,
salary INTEGER NOT NULL,
ssn TEXT NOT NULL
)
`);

// Create a shadow users table (exfiltrable via UNION injection)
dbInstance.run(`
CREATE TABLE IF NOT EXISTS users (
id INTEGER PRIMARY KEY,
username TEXT NOT NULL,
password TEXT NOT NULL,
salary INTEGER,
is_admin INTEGER DEFAULT 0
)
`);

// Seed employees
const employees = [
["Alice Johnson", "Engineering", 95000, "123-45-6789"],
["Bob Smith", "Marketing", 72000, "987-65-4321"],
["Carol White", "HR", 68000, "456-78-9012"],
["David Brown", "Finance", 88000, "321-54-9876"],
["Eve Davis", "Engineering", 102000, "654-32-1098"]
];
const insertEmp = dbInstance.prepare(
"INSERT INTO employees (name, department, salary, ssn) VALUES (?, ?, ?, ?)"
);
employees.forEach(e => insertEmp.run(e));
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 SQL injection in sqlite3 via string-based query concatenation - high severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. To autofix all SQL injection instances in your entire app, install Zen for Node.js.

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

insertEmp.finalize();

// Seed users (simulates credential store accessible via UNION injection)
const users = [
[1, "admin", "s3cr3tAdmin!", 0, 1],
[2, "user1", "Password123", 95000, 0],
[3, "user2", "qwerty", 72000, 0]
];
const insertUser = dbInstance.prepare(
"INSERT INTO users (id, username, password, salary, is_admin) VALUES (?, ?, ?, ?, ?)"
);
users.forEach(u => insertUser.run(u));
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 SQL injection in sqlite3 via string-based query concatenation - high severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. To autofix all SQL injection instances in your entire app, install Zen for Node.js.

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

insertUser.finalize();
});

return dbInstance;
}

/* ReportsDAO provides payroll search functionality */
function ReportsDAO() {
"use strict";

if (false === (this instanceof ReportsDAO)) {
console.log("Warning: ReportsDAO constructor called without 'new' operator");
return new ReportsDAO();
}

const db = getDb();

/*
* VULNERABLE: searchEmployees builds a query via string concatenation.
* The `name` parameter comes directly from req.query.name with no
* sanitization or parameterization, allowing classic SQL injection.
*
* Fix (A1): Use a parameterized query instead:
* const query = "SELECT id, name, department, salary FROM employees WHERE name LIKE ?";
* db.all(query, [`%${name}%`], callback);
*/
this.searchEmployees = (name, callback) => {
// Insecure: user input concatenated directly into SQL string
const query = `SELECT id, name, department, salary FROM employees WHERE name LIKE '%${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.

Potential SQL injection via string-based query concatenation - critical severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. To autofix all SQL injection instances in your entire app, install Zen for Node.js.

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


console.log(`[ReportsDAO] Executing query: ${query}`);

Check warning on line 103 in app/data/reports-dao.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

app/data/reports-dao.js#L103

Detect console.log() with non Literal argument

db.all(query, (err, rows) => {
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 SQL injection in sqlite3 via string-based query concatenation - high severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. To autofix all SQL injection instances in your entire app, install Zen for Node.js.

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

if (err) {
return callback(err, null);
}
return callback(null, rows);
});
};

/*
* VULNERABLE: getEmployeeById fetches a single employee by ID using
* string interpolation. An attacker can inject UNION SELECT to exfiltrate
* data from other tables.
*
* Payload: 0 UNION SELECT id, username, password, is_admin FROM users--
*
* Fix (A1): Use parameterized query:
* db.get("SELECT * FROM employees WHERE id = ?", [id], callback);
*/
this.getEmployeeById = (id, callback) => {
// Insecure: id from request URL parameter concatenated into query
const query = `SELECT * FROM employees WHERE id = ${id}`;
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 SQL injection via string-based query concatenation - critical severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. To autofix all SQL injection instances in your entire app, install Zen for Node.js.

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


console.log(`[ReportsDAO] Executing query: ${query}`);

db.get(query, (err, row) => {
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 SQL injection in sqlite3 via string-based query concatenation - high severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. To autofix all SQL injection instances in your entire app, install Zen for Node.js.

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

if (err) {
return callback(err, null);
}
return callback(null, row);
});
};
}

module.exports = { ReportsDAO };
6 changes: 6 additions & 0 deletions app/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const ContributionsHandler = require("./contributions");
const AllocationsHandler = require("./allocations");
const MemosHandler = require("./memos");
const ResearchHandler = require("./research");
const ReportsHandler = require("./reports");
const tutorialRouter = require("./tutorial");
const ErrorHandler = require("./error").errorHandler;

Expand All @@ -19,6 +20,7 @@ const index = (app, db) => {
const allocationsHandler = new AllocationsHandler(db);
const memosHandler = new MemosHandler(db);
const researchHandler = new ResearchHandler(db);
const reportsHandler = new ReportsHandler();

// Middleware to check if a user is logged in
const isLoggedIn = sessionHandler.isLoggedInMiddleware;
Expand Down Expand Up @@ -75,6 +77,10 @@ const index = (app, db) => {
// Research Page
app.get("/research", isLoggedIn, researchHandler.displayResearch);

// Reports Page - A1: SQL Injection via SQLite string concatenation
app.get("/reports", isLoggedIn, reportsHandler.searchEmployees);
app.get("/reports/employee/:id", isLoggedIn, reportsHandler.getEmployee);

// Mount tutorial router
app.use("/tutorial", tutorialRouter);

Expand Down
90 changes: 90 additions & 0 deletions app/routes/reports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
const { ReportsDAO } = require("../data/reports-dao");
const { environmentalScripts } = require("../../config/config");

function ReportsHandler() {
"use strict";

const reportsDAO = new ReportsDAO();

this.displayReports = (req, res, next) => {
const { userId } = req.session;

return res.render("payroll", {
userId,
employees: null,
searchName: "",
environmentalScripts
});
Comment on lines +12 to +17
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() - high 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

};

/*
* A1 - SQL Injection
* The search term from req.query.name is passed directly to ReportsDAO.searchEmployees
* which concatenates it into a raw SQL string.
*
* Attack: search for ' OR '1'='1 to dump all records.
* Attack: search for ' UNION SELECT id,username,password,salary,0 FROM users--
* to exfiltrate the users table via a UNION-based injection.
*
* Fix: sanitize/validate input before passing to DAO, or use parameterized
* queries in the DAO layer (see comments in reports-dao.js).
*/
this.searchEmployees = (req, res, next) => {
const { userId } = req.session;
// Insecure: raw query parameter forwarded to DAO without sanitization
const searchName = req.query.name || "";

reportsDAO.searchEmployees(searchName, (err, employees) => {
if (err) {
// Surface the raw DB error so attackers can observe schema info (A6)
return res.render("payroll", {
userId,
employees: [],
searchName,
dbError: err.message,
environmentalScripts
});
Comment on lines +40 to +46
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() - high 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

}

return res.render("payroll", {
userId,
employees,
searchName,
environmentalScripts
});
Comment on lines +49 to +54
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() - high 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

});
};

/*
* A1 - SQL Injection (second-order / numeric injection)
* The :id URL parameter is interpolated directly into a SQL query in the DAO.
*
* Attack: GET /reports/employee/0 UNION SELECT id,username,password,salary,0 FROM users--
*/
this.getEmployee = (req, res, next) => {
const { userId } = req.session;
// Insecure: raw URL parameter passed to DAO without parseInt or validation
const empId = req.params.id;

reportsDAO.getEmployeeById(empId, (err, employee) => {
if (err) {
return res.render("payroll", {
userId,
employees: [],
searchName: "",
dbError: err.message,
environmentalScripts
});
Comment on lines +71 to +77
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() - high 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

}

return res.render("payroll", {
userId,
employees: employee ? [employee] : [],
searchName: "",
environmentalScripts
});
Comment on lines +80 to +85
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() - high 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

});
};
}

module.exports = ReportsHandler;
2 changes: 2 additions & 0 deletions app/views/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
</li>
<li><a id="research-menu-link" href="/research"><i class="fa fa-table"></i> Research</a>
</li>
<li><a id="reports-menu-link" href="/reports"><i class="fa fa-file-text"></i> Reports</a>
</li>
{% endif %}
<li><a id="logout-menu-link" href="/logout"><i class="fa fa-power-off"></i> Logout</a>
</li>
Expand Down
82 changes: 82 additions & 0 deletions app/views/payroll.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{% extends "./layout.html" %} {% block title %}Payroll Reports{% endblock %} {% block content %}

<div class="row">
<div class="col-lg-12">

<div class="panel panel-default">
<div class="panel-heading">
<h3 class="panel-title">Employee Payroll Search</h3>
</div>
<div class="panel-body">

<!-- A1 - SQL Injection: search input is concatenated into raw SQL -->
<form action="/reports" method="get" role="search">
<div class="input-group">
<input type="text"
class="form-control"
name="name"
placeholder="Search by employee name..."
value="{{ searchName }}">
<span class="input-group-btn">
<button class="btn btn-primary" type="submit">
<i class="fa fa-search"></i> Search
</button>
</span>
</div>
<p class="help-block">
<strong>Hint (A1 - SQL Injection):</strong>
Try searching for <code>' OR '1'='1</code> to dump all records, or
<code>' UNION SELECT id,username,password,salary,0 FROM users--</code>
to exfiltrate the users table.
</p>
</form>

</div>
</div>

{% if dbError %}
<div class="alert alert-danger">
<strong>Database Error:</strong> {{ dbError }}
</div>
{% endif %}

{% if employees %}
<div class="panel panel-info">
<div class="panel-heading">
<h3 class="panel-title">Results</h3>
</div>
<div class="panel-body">
<table class="table table-bordered table-hover">
<thead>
<tr>
<th>ID</th>
<th>Name</th>
<th>Department</th>
<th>Salary</th>
<th>Actions</th>
</tr>
</thead>
<tbody>
{% for emp in employees %}
<tr>
<td>{{ emp.id }}</td>
<td>{{ emp.name }}</td>
<td>{{ emp.department }}</td>
<td>${{ emp.salary }}</td>
<td>
<!-- A1 - numeric injection: id goes straight into SQL -->
<a href="/reports/employee/{{ emp.id }}" class="btn btn-xs btn-info">
View Detail
</a>
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
</div>
{% endif %}

</div>
</div>
{% endblock %}
17 changes: 15 additions & 2 deletions package.json
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

15 Open source vulnerabilities detected - critical severity
Aikido detected 15 vulnerabilities across 4 packages, it includes 5 critical, 8 high, 1 medium and 1 low vulnerabilities.

Details

Remediation Aikido suggests bumping the vulnerable packages to a safe version.

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

Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,27 @@
"helmet": "^2.0.0",
"marked": "0.3.5",
"mongodb": "^2.1.18",
"sqlite3": "^5.1.6",

Check warning on line 19 in package.json

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

package.json#L19

Package dependencies with variant versions may lead to dependency hijack and confusion attacks.
"needle": "2.2.4",
"node-esapi": "0.0.1",
"serve-favicon": "^2.3.0",
"swig": "^1.4.2",
"underscore": "^1.8.3"
"underscore": "^1.8.3",

Check warning on line 24 in package.json

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

package.json#L24

Package dependencies with variant versions may lead to dependency hijack and confusion attacks.
"lodash": "4.17.4",
"minimist": "0.0.8",
"handlebars": "4.0.11",
"serialize-javascript": "2.1.1",
"node-uuid": "1.4.7"
},
"comments": {
"//": "a9 insecure components"
"//": "a9 insecure components",
"vulnerable-packages": {
"lodash@4.17.4": "CVE-2019-10744 - prototype pollution via _.defaultsDeep / _.merge / _.mergeWith",
"minimist@0.0.8": "CVE-2020-7598 - prototype pollution via --__proto__ CLI flag parsing",
"handlebars@4.0.11": "CVE-2019-20920 / CVE-2019-19919 - prototype pollution + RCE via template compilation",
"serialize-javascript@2.1.1": "CVE-2019-16769 - XSS via unescaped </script> in serialized regexes",
"node-uuid@1.4.7": "insecure random number generation; replaced by 'uuid' package"
}
},
"scripts": {
"start": "node server.js",
Expand Down
Loading