Fix upload path-traversal RCE vector, patch all known-vulnerable deps
Path traversal (CWE-22/CWE-73): event-image, branding (logo/favicon), and event-attachment uploads built the saved filename from the client-supplied original filename with no sanitization, and multer's diskStorage joins that straight into the destination path. A crafted filename containing `../` sequences could write the uploaded file anywhere the server process has write access — reachable by any supervisor-level account, and briefly pre-auth via the branding uploads during initial /setup. Filenames are now always server- generated (random bytes + validated extension); the original name is kept only as display metadata. Dependencies: express-rate-limit was declared only at the repo root despite being required directly by backend/src/index.js, so a plain `cd backend && npm install` (per the deployment doc) would never install it — moved it into backend/package.json. Bumped next off a version affected by a critical unauthenticated RCE (React Flight protocol) and switched it from an exact pin to a caret range so future patches install automatically. Bumped multer/nodemailer/jsonwebtoken/ uuid to patched versions, with an override forcing the vulnerable nested uuid inside exceljs and the vulnerable postcss bundled inside next to the patched versions too. `npm audit` is now clean (0 vulnerabilities) across root, backend, and frontend. Hardening: jwt.verify() now pins algorithms: ['HS256'] instead of trusting the token header; /uploads now serves with a restrictive CSP and X-Content-Type-Options: nosniff so an uploaded SVG containing <script> can't execute if opened directly. Verified: backend's Jest suite passes, the backend boots and serves real requests on the bumped deps, and `next build` compiles/type- checks cleanly on the bumped frontend deps. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSWFWQsjTc9GyffPiXEDQT
This commit is contained in:
@@ -3,6 +3,7 @@ const { v4: uuidv4 } = require('uuid');
|
||||
const multer = require('multer');
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
const crypto = require('crypto');
|
||||
const { assertEventOpen } = require('../utils/cashupUtils');
|
||||
const { logAdminAction } = require('../utils/adminAudit');
|
||||
const { getClientIp } = require('../utils/requestUtils');
|
||||
@@ -987,7 +988,10 @@ const attachmentsStorage = multer.diskStorage({
|
||||
}
|
||||
},
|
||||
filename: function (req, file, cb) {
|
||||
const unique = `${Date.now()}-${file.originalname}`;
|
||||
// Extension only — file.originalname is untrusted and joining it into a
|
||||
// path allows `../` traversal to write outside the upload directory.
|
||||
const ext = path.extname(file.originalname).toLowerCase();
|
||||
const unique = `event-file-${Date.now()}-${crypto.randomBytes(8).toString('hex')}${ext}`;
|
||||
cb(null, unique);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1,7 +1,15 @@
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
const crypto = require('crypto');
|
||||
const multer = require('multer');
|
||||
|
||||
// Builds a filename multer can never be tricked into escaping the upload
|
||||
// directory with — extension only, no attacker-controlled path segments.
|
||||
// (file.originalname is untrusted; joining it into a path allows `../` traversal.)
|
||||
function safeFilename(prefix, ext) {
|
||||
return `${prefix}-${Date.now()}-${crypto.randomBytes(8).toString('hex')}${ext}`;
|
||||
}
|
||||
|
||||
// Setup multer storage
|
||||
const storage = multer.diskStorage({
|
||||
destination: function (req, file, cb) {
|
||||
@@ -24,8 +32,7 @@ const storage = multer.diskStorage({
|
||||
}
|
||||
},
|
||||
filename: function (req, file, cb) {
|
||||
const uniqueName = `${Date.now()}-${file.originalname}`;
|
||||
cb(null, uniqueName);
|
||||
cb(null, safeFilename('event', path.extname(file.originalname).toLowerCase()));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -54,7 +61,7 @@ const logoStorage = multer.diskStorage({
|
||||
}
|
||||
},
|
||||
filename: function (req, file, cb) {
|
||||
cb(null, `logo-${Date.now()}${path.extname(file.originalname).toLowerCase()}`);
|
||||
cb(null, safeFilename('logo', path.extname(file.originalname).toLowerCase()));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -82,7 +89,7 @@ const faviconStorage = multer.diskStorage({
|
||||
}
|
||||
},
|
||||
filename: function (req, file, cb) {
|
||||
cb(null, `favicon-${Date.now()}${path.extname(file.originalname).toLowerCase()}`);
|
||||
cb(null, safeFilename('favicon', path.extname(file.originalname).toLowerCase()));
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
+11
-2
@@ -177,7 +177,16 @@ app.use('/api/backups', backupRoutes);
|
||||
// Pre-warm the settings cache so synchronous helpers have DB values from startup
|
||||
const { getSettingSync, warmCache } = require('./utils/settingsCache');
|
||||
warmCache().catch(() => {});
|
||||
app.use('/uploads', express.static('public/uploads'));
|
||||
// Uploaded branding assets can include SVGs, which may embed <script>/event
|
||||
// handlers. Serving them inline lets a compromised/malicious upload run script
|
||||
// in the site's origin if opened directly, so pin the safe response headers
|
||||
// (no inline execution, no MIME-sniffing to HTML/script) on every asset here.
|
||||
app.use('/uploads', express.static('public/uploads', {
|
||||
setHeaders: (res) => {
|
||||
res.setHeader('Content-Security-Policy', "default-src 'none'; style-src 'unsafe-inline'; sandbox");
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
},
|
||||
}));
|
||||
|
||||
// ── Shared page helpers ────────────────────────────────────────────────────────
|
||||
const jwt = require('jsonwebtoken');
|
||||
@@ -347,7 +356,7 @@ app.get('/docs', async (req, res) => {
|
||||
|
||||
let user;
|
||||
try {
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET);
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET, { algorithms: ['HS256'] });
|
||||
user = await prisma.user.findUnique({
|
||||
where: { id: decoded.id },
|
||||
select: { id: true, name: true, email: true, role: true, isActive: true, tokenVersion: true },
|
||||
|
||||
@@ -13,8 +13,9 @@ const protect = async (req, res, next) => {
|
||||
// Get token from header
|
||||
token = req.headers.authorization.split(' ')[1];
|
||||
|
||||
// Verify token
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET);
|
||||
// Verify token — pin the algorithm so a token signed with an
|
||||
// unexpected/attacker-chosen algorithm is never accepted.
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET, { algorithms: ['HS256'] });
|
||||
|
||||
// Get user from the token (exclude password)
|
||||
req.user = await prisma.user.findUnique({
|
||||
@@ -105,7 +106,7 @@ const optionalAuth = async (req, res, next) => {
|
||||
}
|
||||
try {
|
||||
const token = req.headers.authorization.split(' ')[1];
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET);
|
||||
const decoded = jwt.verify(token, process.env.JWT_SECRET, { algorithms: ['HS256'] });
|
||||
const user = await prisma.user.findUnique({
|
||||
where: { id: decoded.id },
|
||||
select: { id: true, name: true, email: true, role: true, isActive: true, createdAt: true, updatedAt: true, phoneNumber: true, tokenVersion: true }
|
||||
|
||||
Reference in New Issue
Block a user