Fix remaining critical, medium, and low security issues
IDOR (critical): GET /api/checks/:id and GET /api/deposits/:id now verify the requesting user has access to the record's account before returning data. Previously any authenticated user could fetch any record by ID across accounts. Printed check guard (critical): PUT and DELETE on checks now return 409 if the check has already been printed, enforcing the business rule that printed checks are immutable. Previously the printed flag was only enforced in the frontend. PDF DoS (medium): checkIds array capped at 300 (100 pages × 3 per page). QBO import DoS (medium): records array capped at 1000 per confirm call. PDF error detail (medium): internal err.message no longer returned to the client on PDF generation failure. SESSION_SECRET (low): removed NODE_ENV=production condition — the server now exits immediately on startup if SESSION_SECRET is unset regardless of environment. Dev script updated to load .env via node --env-file=.env so developers set it once in a local .env file. Password hints (low): updated all three UI labels from "min 8 chars" to "min 10 chars, include a digit or symbol" to match the actual server-side validation.
This commit is contained in:
+1
-1
@@ -5,7 +5,7 @@
|
||||
"main": "src/app.js",
|
||||
"scripts": {
|
||||
"start": "node src/app.js",
|
||||
"dev": "nodemon src/app.js",
|
||||
"dev": "nodemon --exec \"node --env-file=.env\" src/app.js",
|
||||
"migrate": "node migrations/import-mdb.js"
|
||||
},
|
||||
"dependencies": {
|
||||
|
||||
+3
-3
@@ -20,7 +20,7 @@
|
||||
<input type="text" id="setup-username" autocomplete="username" autocapitalize="none">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label for="setup-password">Password <span class="field-hint">(min 8 characters)</span></label>
|
||||
<label for="setup-password">Password <span class="field-hint">(min 10 characters, include a digit or symbol)</span></label>
|
||||
<input type="password" id="setup-password" autocomplete="new-password">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
@@ -621,7 +621,7 @@
|
||||
<input type="text" id="uf-username" autocapitalize="none">
|
||||
</div>
|
||||
<div class="form-group required">
|
||||
<label for="uf-password">Password <span class="field-hint" id="uf-password-hint">(min 8 chars)</span></label>
|
||||
<label for="uf-password">Password <span class="field-hint" id="uf-password-hint">(min 10 chars, include a digit or symbol)</span></label>
|
||||
<input type="password" id="uf-password" autocomplete="new-password">
|
||||
</div>
|
||||
<div class="form-group required">
|
||||
@@ -652,7 +652,7 @@
|
||||
<input type="password" id="cp-current" autocomplete="current-password">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label for="cp-new">New Password <span class="field-hint">(min 8 chars)</span></label>
|
||||
<label for="cp-new">New Password <span class="field-hint">(min 10 chars, include a digit or symbol)</span></label>
|
||||
<input type="password" id="cp-new" autocomplete="new-password">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
|
||||
+3
-4
@@ -18,12 +18,11 @@ const upload = multer({ dest: os.tmpdir() });
|
||||
// ── Session store (SQLite-backed, no extra packages) ──────────────────────────
|
||||
const SessionStore = require('./lib/SessionStore');
|
||||
|
||||
if (!process.env.SESSION_SECRET && process.env.NODE_ENV === 'production') {
|
||||
console.error('[fatal] SESSION_SECRET environment variable must be set in production. Exiting.');
|
||||
if (!process.env.SESSION_SECRET) {
|
||||
console.error('[fatal] SESSION_SECRET environment variable is not set. See .env.example. Exiting.');
|
||||
process.exit(1);
|
||||
}
|
||||
const SESSION_SECRET = process.env.SESSION_SECRET ||
|
||||
(() => { console.warn('[warn] SESSION_SECRET not set — using random secret (sessions will reset on restart)'); return crypto.randomBytes(32).toString('hex'); })();
|
||||
const SESSION_SECRET = process.env.SESSION_SECRET;
|
||||
|
||||
const SESSION_MAX_AGE_MS = (parseInt(process.env.SESSION_MAX_AGE_HOURS, 10) || 168) * 60 * 60 * 1000;
|
||||
|
||||
|
||||
@@ -41,6 +41,9 @@ router.get('/', (req, res) => {
|
||||
router.get('/:id', (req, res) => {
|
||||
const check = db.prepare('SELECT * FROM checks WHERE id = ?').get(req.params.id);
|
||||
if (!check) return res.status(404).json({ error: 'Check not found' });
|
||||
if (!canAccessAccount(req.session, check.account_id)) {
|
||||
return res.status(403).json({ error: 'Access denied.' });
|
||||
}
|
||||
res.json(check);
|
||||
});
|
||||
|
||||
@@ -99,6 +102,7 @@ router.put('/:id', (req, res) => {
|
||||
if (!isEditorForAccount(req.session, check.account_id)) {
|
||||
return res.status(403).json({ error: 'Write access required.' });
|
||||
}
|
||||
if (check.printed) return res.status(409).json({ error: 'Cannot modify a printed check.' });
|
||||
|
||||
const { payee, amount, check_date, memo, note1, note2,
|
||||
payee_address1, payee_address2, payee_address3, payee_address4 } = req.body;
|
||||
@@ -140,6 +144,7 @@ router.delete('/:id', (req, res) => {
|
||||
if (!isEditorForAccount(req.session, check.account_id)) {
|
||||
return res.status(403).json({ error: 'Write access required.' });
|
||||
}
|
||||
if (check.printed) return res.status(409).json({ error: 'Cannot delete a printed check.' });
|
||||
db.prepare('DELETE FROM checks WHERE id = ?').run(req.params.id);
|
||||
res.status(204).send();
|
||||
});
|
||||
|
||||
@@ -38,6 +38,9 @@ router.get('/', (req, res) => {
|
||||
router.get('/:id', (req, res) => {
|
||||
const deposit = getDepositWithItems(req.params.id);
|
||||
if (!deposit) return res.status(404).json({ error: 'Deposit not found.' });
|
||||
if (!canAccessAccount(req.session, deposit.account_id)) {
|
||||
return res.status(403).json({ error: 'Access denied.' });
|
||||
}
|
||||
res.json(deposit);
|
||||
});
|
||||
|
||||
|
||||
+5
-1
@@ -17,9 +17,13 @@ const { isEditorForAccount } = require('../middleware/auth');
|
||||
router.post('/', async (req, res) => {
|
||||
const { checkIds, account_id } = req.body;
|
||||
|
||||
const MAX_CHECKS_PER_JOB = 300; // 100 pages × 3 checks per page
|
||||
if (!Array.isArray(checkIds) || checkIds.length === 0) {
|
||||
return res.status(400).json({ error: 'checkIds must be a non-empty array' });
|
||||
}
|
||||
if (checkIds.length > MAX_CHECKS_PER_JOB) {
|
||||
return res.status(400).json({ error: `Cannot generate more than ${MAX_CHECKS_PER_JOB} checks per PDF job.` });
|
||||
}
|
||||
const resolvedAccountId = parseInt(account_id, 10);
|
||||
if (!isEditorForAccount(req.session, resolvedAccountId)) {
|
||||
return res.status(403).json({ error: 'Write access required.' });
|
||||
@@ -62,7 +66,7 @@ router.post('/', async (req, res) => {
|
||||
res.send(pdfBuffer);
|
||||
} catch (err) {
|
||||
console.error('PDF generation error:', err);
|
||||
res.status(500).json({ error: 'PDF generation failed', detail: err.message });
|
||||
res.status(500).json({ error: 'PDF generation failed.' });
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -309,6 +309,9 @@ router.post('/confirm', express.json(), (req, res) => {
|
||||
if (!Array.isArray(records) || records.length === 0) {
|
||||
return res.status(400).json({ error: 'No records provided.' });
|
||||
}
|
||||
if (records.length > 1000) {
|
||||
return res.status(400).json({ error: 'Cannot import more than 1000 records at a time.' });
|
||||
}
|
||||
|
||||
const db = require('../db/database');
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user