jjkiers/cloudron-box
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Fix crash when req.query handling

Browse Source 
https://expressjs.com/en/5x/api.html#req.query

"As req.query’s shape is based on user-controlled input, all properties and values in this object
are untrusted and should be validated before trusting"

In essence, req.query.xx can be an array OR an array of strings.
This commit is contained in:
Girish Ramakrishnan
2025-07-13 13:14:32 +02:00
parent dc7f5e3dbc
commit 04de621e37
14 changed files with 66 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/routes/accesscontrol.js
+3 -3
View File
@@ -43,9 +43,9 @@ async function tokenAuth(req, res, next) {
let accessToken;
// this determines the priority
if (req.body && req.body.access_token) accessToken = req.body.access_token;
if (req.query && req.query.access_token) accessToken = req.query.access_token;
if (req.headers && req.headers.authorization) {
if (req.body?.access_token) accessToken = req.body.access_token;
if (typeof req.query?.access_token === 'string') accessToken = req.query.access_token;
if (req.headers?.authorization) {
const parts = req.headers.authorization.split(' ');
if (parts.length == 2) {
const [scheme, credentials] = parts;
src/routes/apps.js
+20 -17
View File
@@ -127,7 +127,9 @@ async function listByUser(req, res, next) {
async function getAppIcon(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
const [error, icon] = await safe(apps.getIcon(req.resources.app, { original: req.query.original }));
const original = typeof req.query.original === 'string' ? (req.query.original === '1' || req.query.original === 'true') : false;
const [error, icon] = await safe(apps.getIcon(req.resources.app, { original }));
if (error) return next(BoxError.toHttpError(error));
res.send(icon);
@@ -721,7 +723,7 @@ async function update(req, res, next) {
async function getLogStream(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a valid number'));
if (req.headers.accept !== 'text/event-stream') return next(new HttpError(400, 'This API call requires EventStream'));
@@ -756,13 +758,13 @@ async function getLogStream(req, res, next) {
async function getLogs(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10;
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10;
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a number'));
const options = {
lines,
follow: false,
format: req.query.format || 'json'
format: typeof req.query.format === 'string' ? req.query.format : 'json'
};
const [error, logStream] = await safe(apps.getLogs(req.resources.app, options));
@@ -826,13 +828,13 @@ async function startExec(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
assert.strictEqual(typeof req.params.execId, 'string');
const columns = req.query.columns ? parseInt(req.query.columns, 10) : null;
const columns = typeof req.query.columns === 'string' ? parseInt(req.query.columns, 10) : null;
if (isNaN(columns)) return next(new HttpError(400, 'columns must be a number'));
const rows = req.query.rows ? parseInt(req.query.rows, 10) : null;
const rows = typeof req.query.rows === 'string' ? parseInt(req.query.rows, 10) : null;
if (isNaN(rows)) return next(new HttpError(400, 'rows must be a number'));
const tty = req.query.tty === 'true';
const tty = typeof req.query.tty === 'string' ? (req.query.tty === '1' || req.query.tty === 'true') : false;
if (safe.query(req.resources.app, 'manifest.addons.docker') && req.user.role !== users.ROLE_OWNER) return next(new HttpError(403, '"owner" role is requied to exec app with docker addon'));
@@ -861,13 +863,13 @@ async function startExecWebSocket(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
assert.strictEqual(typeof req.params.execId, 'string');
const columns = req.query.columns ? parseInt(req.query.columns, 10) : null;
const columns = typeof req.query.columns === 'string' ? parseInt(req.query.columns, 10) : null;
if (isNaN(columns)) return next(new HttpError(400, 'columns must be a number'));
const rows = req.query.rows ? parseInt(req.query.rows, 10) : null;
const rows = typeof req.query.rows === 'string' ? parseInt(req.query.rows, 10) : null;
if (isNaN(rows)) return next(new HttpError(400, 'rows must be a number'));
const tty = req.query.tty === 'true' ? true : false;
const tty = typeof req.query.tty === 'string' ? (req.query.tty === '1' || req.query.tty === 'true') : false;
// in a badly configured reverse proxy, we might be here without an upgrade
if (req.headers['upgrade'] !== 'websocket') return next(new HttpError(404, 'exec requires websocket'));
@@ -912,10 +914,10 @@ async function getExec(req, res, next) {
async function listBackups(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
const [error, result] = await safe(apps.listBackups(req.resources.app, page, perPage));
@@ -1032,10 +1034,10 @@ async function setUpstreamUri(req, res, next) {
async function listEventlog(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
const [error, eventlogs] = await safe(apps.listEventlog(req.resources.app, page, perPage));
@@ -1070,12 +1072,13 @@ async function getTask(req, res, next) {
async function getMetrics(req, res, next) {
assert.strictEqual(typeof req.resources.app, 'object');
if (!req.query.fromSecs || !parseInt(req.query.fromSecs)) return next(new HttpError(400, 'fromSecs must be a number'));
if (!req.query.intervalSecs || !parseInt(req.query.intervalSecs)) return next(new HttpError(400, 'intervalSecs must be a number'));
if (typeof req.query.fromSecs !== 'string' || !parseInt(req.query.fromSecs)) return next(new HttpError(400, 'fromSecs must be a number'));
if (typeof req.query.intervalSecs !== 'string' || !parseInt(req.query.intervalSecs)) return next(new HttpError(400, 'intervalSecs must be a number'));
const fromSecs = parseInt(req.query.fromSecs);
const intervalSecs = parseInt(req.query.intervalSecs);
const noNullPoints = !!req.query.noNullPoints;
const noNullPoints = typeof req.query.noNullPoints === 'string' ? (req.query.noNullPoints === '1' || req.query.noNullPoints === 'true') : false;
const [error, result] = await safe(metrics.get({ fromSecs, noNullPoints, intervalSecs, appIds: [req.resources.app.id] }));
if (error) return next(new HttpError(500, error));
src/routes/archives.js
+5 -3
View File
@@ -32,10 +32,10 @@ async function load(req, res, next) {
}
async function list(req, res, next) {
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
const [error, result] = await safe(archives.list(page, perPage));
@@ -54,7 +54,9 @@ async function getIcon(req, res, next) {
assert.strictEqual(typeof req.params.id, 'string');
assert.strictEqual(typeof req.resources.archive, 'object');
const [error, icon] = await safe(archives.getIcon(req.params.id, { original: req.query.original }));
const original = typeof req.query.original === 'string' ? (req.query.original === '1' || req.query.original === 'true') : false;
const [error, icon] = await safe(archives.getIcon(req.params.id, { original }));
if (error) return next(BoxError.toHttpError(error));
res.send(icon);
src/routes/backups.js
+2 -2
View File
@@ -25,10 +25,10 @@ const assert = require('assert'),
safe = require('safetydance');
async function list(req, res, next) {
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
const [error, result] = await safe(backups.getByIdentifierAndStatePaged(backups.BACKUP_IDENTIFIER_BOX, backups.BACKUP_STATE_NORMAL, page, perPage));
src/routes/domains.js
+1 -1
View File
@@ -145,7 +145,7 @@ async function del(req, res, next) {
async function checkDnsRecords(req, res, next) {
assert.strictEqual(typeof req.params.domain, 'string');
if (!('subdomain' in req.query)) return next(new HttpError(400, 'subdomain is required'));
if (typeof req.query.subdomain !=='string') return next(new HttpError(400, 'subdomain is required'));
let [error, result] = await safe(domains.get(req.params.domain));
if (error) return next(BoxError.toHttpError(error));
src/routes/eventlog.js
+3 -3
View File
@@ -20,17 +20,17 @@ async function get(req, res, next) {
}
async function list(req, res, next) {
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
if (req.query.actions && typeof req.query.actions !== 'string') return next(new HttpError(400, 'actions must be a comma separated string'));
if (req.query.action && typeof req.query.action !== 'string') return next(new HttpError(400, 'action must be a string'));
if (req.query.search && typeof req.query.search !== 'string') return next(new HttpError(400, 'search must be a string'));
const actions = req.query.actions ? req.query.actions.split(',').map(function (s) { return s.trim(); }) : [];
const actions = typeof req.query.actions === 'string' ? req.query.actions.split(',').map(function (s) { return s.trim(); }) : [];
if (req.query.action) actions.push(req.query.action);
const [error, eventlogs] = await safe(eventlog.listPaged(actions, req.query.search || null, page, perPage));
src/routes/mail.js
+4 -4
View File
@@ -138,10 +138,10 @@ async function sendTestMail(req, res, next) {
async function listMailboxes(req, res, next) {
assert.strictEqual(typeof req.params.domain, 'string');
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a positive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a positive number'));
if (req.query.search && typeof req.query.search !== 'string') return next(new HttpError(400, 'search must be a string'));
@@ -269,10 +269,10 @@ async function setBanner(req, res, next) {
async function getLists(req, res, next) {
assert.strictEqual(typeof req.params.domain, 'string');
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a positive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a positive number'));
if (req.query.search && typeof req.query.search !== 'string') return next(new HttpError(400, 'search must be a string'));
src/routes/notifications.js
+2 -2
View File
@@ -33,10 +33,10 @@ function get(req, res, next) {
}
async function list(req, res, next) {
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
if (req.query.acknowledged && !(req.query.acknowledged === 'true' || req.query.acknowledged === 'false')) return next(new HttpError(400, 'acknowledged must be a true or false'));
src/routes/services.js
+6 -6
View File
@@ -60,13 +60,13 @@ async function configure(req, res, next) {
async function getLogs(req, res, next) {
assert.strictEqual(typeof req.params.service, 'string');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a number'));
const options = {
lines: lines,
follow: false,
format: req.query.format || 'json'
format: typeof req.query.format === 'string' ? req.query.format : 'json'
};
const [error, logStream] = await safe(services.getServiceLogs(req.params.service, options));
@@ -86,7 +86,7 @@ async function getLogs(req, res, next) {
async function getLogStream(req, res, next) {
assert.strictEqual(typeof req.params.service, 'string');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a valid number'));
if (req.headers.accept !== 'text/event-stream') return next(new HttpError(400, 'This API call requires EventStream'));
@@ -139,12 +139,12 @@ async function rebuild(req, res, next) {
async function getMetrics(req, res, next) {
assert.strictEqual(typeof req.params.service, 'string');
if (!req.query.fromSecs || !parseInt(req.query.fromSecs)) return next(new HttpError(400, 'fromSecs must be a number'));
if (!req.query.intervalSecs || !parseInt(req.query.intervalSecs)) return next(new HttpError(400, 'intervalSecs must be a number'));
if (typeof req.query.fromSecs !== 'string' || !parseInt(req.query.fromSecs)) return next(new HttpError(400, 'fromSecs must be a number'));
if (typeof req.query.intervalSecs !== 'string' || !parseInt(req.query.intervalSecs)) return next(new HttpError(400, 'intervalSecs must be a number'));
const fromSecs = parseInt(req.query.fromSecs);
const intervalSecs = parseInt(req.query.intervalSecs);
const noNullPoints = !!req.query.noNullPoints;
const noNullPoints = typeof req.query.noNullPoints === 'string' ? (req.query.noNullPoints === '1' || req.query.noNullPoints === 'true') : false;
const [error, result] = await safe(metrics.get({ fromSecs, intervalSecs, noNullPoints, serviceIds: [req.params.service] }));
if (error) return next(new HttpError(500, error));
src/routes/system.js
+7 -7
View File
@@ -60,13 +60,13 @@ async function getMemory(req, res, next) {
async function getLogs(req, res, next) {
assert.strictEqual(typeof req.params.unit, 'string');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a number'));
const options = {
lines: lines,
follow: false,
format: req.query.format || 'json'
format: typeof req.query.format === 'string' ? req.query.format : 'json'
};
const [error, logStream] = await safe(system.getLogs(req.params.unit, options));
@@ -85,7 +85,7 @@ async function getLogs(req, res, next) {
async function getLogStream(req, res, next) {
assert.strictEqual(typeof req.params.unit, 'string');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a valid number'));
function sse(id, data) { return 'id: ' + id + '\ndata: ' + data + '\n\n'; }
@@ -95,7 +95,7 @@ async function getLogStream(req, res, next) {
const options = {
lines: lines,
follow: true,
format: req.query.format || 'json'
format: typeof req.query.format === 'string' ? req.query.format : 'json'
};
const [error, logStream] = await safe(system.getLogs(req.params.unit, options));
@@ -119,12 +119,12 @@ async function getLogStream(req, res, next) {
}
async function getMetrics(req, res, next) {
if (!req.query.fromSecs || !parseInt(req.query.fromSecs, 10)) return next(new HttpError(400, 'fromSecs must be a number'));
if (!req.query.intervalSecs || !parseInt(req.query.intervalSecs, 10)) return next(new HttpError(400, 'intervalSecs must be a number'));
if (typeof req.query.fromSecs !== 'string' || !parseInt(req.query.fromSecs, 10)) return next(new HttpError(400, 'fromSecs must be a number'));
if (typeof req.query.intervalSecs !== 'string' || !parseInt(req.query.intervalSecs, 10)) return next(new HttpError(400, 'intervalSecs must be a number'));
const fromSecs = parseInt(req.query.fromSecs, 10);
const intervalSecs = parseInt(req.query.intervalSecs, 10);
const noNullPoints = !!req.query.noNullPoints;
const noNullPoints = typeof req.query.noNullPoints === 'string' ? (req.query.noNullPoints === '1' || req.query.noNullPoints === 'true') : false;
const system = req.query.system === 'true';
const appIds = 'appId' in req.query ? (Array.isArray(req.query.appId) ? req.query.appId : [ req.query.appId ]) : [];
const serviceIds = 'serviceId' in req.query ? (Array.isArray(req.query.serviceId) ? req.query.serviceId : [ req.query.serviceId ]) : [];
src/routes/tasks.js
+5 -5
View File
@@ -37,10 +37,10 @@ async function get(req, res, next) {
}
async function list(req, res, next) {
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined'? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string'? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
if (req.query.type && typeof req.query.type !== 'string') return next(new HttpError(400, 'type must be a string'));
@@ -63,13 +63,13 @@ async function stopTask(req, res, next) {
async function getLogs(req, res, next) {
assert.strictEqual(typeof req.resources.task, 'object');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a number'));
const options = {
lines: lines,
follow: false,
format: req.query.format || 'json'
format: typeof req.query.format === 'string' ? req.query.format : 'json'
};
const [error, logStream] = await safe(tasks.getLogs(req.resources.task, options));
@@ -89,7 +89,7 @@ async function getLogs(req, res, next) {
async function getLogStream(req, res, next) {
assert.strictEqual(typeof req.resources.task, 'object');
const lines = 'lines' in req.query ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
const lines = typeof req.query.lines === 'string' ? parseInt(req.query.lines, 10) : 10; // we ignore last-event-id
if (isNaN(lines)) return next(new HttpError(400, 'lines must be a valid number'));
function sse(id, data) { return 'id: ' + id + '\ndata: ' + data + '\n\n'; }
src/routes/users.js
+3 -3
View File
@@ -155,15 +155,15 @@ async function updateProfile(req, res, next) {
}
async function list(req, res, next) {
const page = typeof req.query.page !== 'undefined' ? parseInt(req.query.page) : 1;
const page = typeof req.query.page === 'string' ? parseInt(req.query.page) : 1;
if (!page || page < 0) return next(new HttpError(400, 'page query param has to be a postive number'));
const perPage = typeof req.query.per_page !== 'undefined' ? parseInt(req.query.per_page) : 25;
const perPage = typeof req.query.per_page === 'string' ? parseInt(req.query.per_page) : 25;
if (!perPage || perPage < 0) return next(new HttpError(400, 'per_page query param has to be a postive number'));
if (req.query.search && typeof req.query.search !== 'string') return next(new HttpError(400, 'search must be a string'));
const active = typeof req.query.active !== 'undefined' ? ((req.query.active === '1' || req.query.active === 'true') ? true : false) : null;
const active = typeof req.query.active === 'string' ? ((req.query.active === '1' || req.query.active === 'true') ? true : false) : null;
const [error, results] = await safe(users.listPaged(req.query.search || null, active, page, perPage));
if (error) return next(BoxError.toHttpError(error));