Fix high and medium security vulnerabilities
CSRF: upgrade session cookie sameSite from 'lax' to 'strict'. Rate limiting: login endpoint now blocks an IP after 10 failed attempts in a 15-minute window; resets on success. In-memory, no new dependency. SESSION_SECRET: server exits at startup when NODE_ENV=production and SESSION_SECRET is unset. docker-compose.yml updated to pass it via env; .env.example added with generation instructions. Security headers: add X-Content-Type-Options, X-Frame-Options, and Referrer-Policy to all responses. Sensitive data: routing_number and account_number are now omitted from GET /api/account/:id responses for non-admin users. Image size: logo upload capped at 512 KB in the account PUT handler. Amount validation: checks (POST/PUT) and deposit items (POST/PUT) now reject non-finite and non-positive amounts. QBO import: uploaded file is rejected if its MIME type is not text or a known CSV variant.
This commit is contained in:
@@ -0,0 +1,6 @@
|
|||||||
|
# Copy to .env and fill in values before starting in production.
|
||||||
|
# Generate SESSION_SECRET with: openssl rand -hex 32
|
||||||
|
|
||||||
|
SESSION_SECRET=replace-with-a-random-64-character-hex-string
|
||||||
|
PORT=3000
|
||||||
|
DB_PATH=/app/data/check-printing.db
|
||||||
@@ -68,6 +68,7 @@ web_modules/
|
|||||||
# dotenv environment variable files
|
# dotenv environment variable files
|
||||||
.env
|
.env
|
||||||
.env.*
|
.env.*
|
||||||
|
!.env.example
|
||||||
|
|
||||||
# parcel-bundler cache (https://parceljs.org/)
|
# parcel-bundler cache (https://parceljs.org/)
|
||||||
.cache
|
.cache
|
||||||
|
|||||||
@@ -12,6 +12,8 @@ services:
|
|||||||
- NODE_ENV=production
|
- NODE_ENV=production
|
||||||
- PORT=3000
|
- PORT=3000
|
||||||
- DB_PATH=/app/data/check-printing.db
|
- DB_PATH=/app/data/check-printing.db
|
||||||
|
# Required in production — generate with: openssl rand -hex 32
|
||||||
|
- SESSION_SECRET=${SESSION_SECRET}
|
||||||
|
|
||||||
volumes:
|
volumes:
|
||||||
check-printing-data:
|
check-printing-data:
|
||||||
|
|||||||
+24
-7
@@ -18,17 +18,29 @@ const upload = multer({ dest: os.tmpdir() });
|
|||||||
// ── Session store (SQLite-backed, no extra packages) ──────────────────────────
|
// ── Session store (SQLite-backed, no extra packages) ──────────────────────────
|
||||||
const SessionStore = require('./lib/SessionStore');
|
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.');
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
const SESSION_SECRET = process.env.SESSION_SECRET ||
|
const SESSION_SECRET = process.env.SESSION_SECRET ||
|
||||||
(() => { console.warn('[warn] SESSION_SECRET not set — using random secret (sessions reset on restart)'); return crypto.randomBytes(32).toString('hex'); })();
|
(() => { console.warn('[warn] SESSION_SECRET not set — using random secret (sessions will reset on restart)'); return crypto.randomBytes(32).toString('hex'); })();
|
||||||
|
|
||||||
app.use(session({
|
app.use(session({
|
||||||
store: new SessionStore(db),
|
store: new SessionStore(db),
|
||||||
secret: SESSION_SECRET,
|
secret: SESSION_SECRET,
|
||||||
resave: false,
|
resave: false,
|
||||||
saveUninitialized: false,
|
saveUninitialized: false,
|
||||||
cookie: { httpOnly: true, sameSite: 'lax', maxAge: 7 * 24 * 60 * 60 * 1000 }, // 7 days
|
cookie: { httpOnly: true, sameSite: 'strict', maxAge: 7 * 24 * 60 * 60 * 1000 }, // 7 days
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
// Security headers
|
||||||
|
app.use((req, res, next) => {
|
||||||
|
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||||
|
res.setHeader('X-Frame-Options', 'DENY');
|
||||||
|
res.setHeader('Referrer-Policy', 'same-origin');
|
||||||
|
next();
|
||||||
|
});
|
||||||
|
|
||||||
app.use(express.json({ limit: '10mb' }));
|
app.use(express.json({ limit: '10mb' }));
|
||||||
app.use(express.static(path.join(__dirname, '../public')));
|
app.use(express.static(path.join(__dirname, '../public')));
|
||||||
|
|
||||||
@@ -91,6 +103,10 @@ app.put('/api/account/:id', requireAdmin, (req, res) => {
|
|||||||
if (!company1 || !routing_number || !account_number) {
|
if (!company1 || !routing_number || !account_number) {
|
||||||
return res.status(400).json({ error: 'Organization name, routing number, and account number are required.' });
|
return res.status(400).json({ error: 'Organization name, routing number, and account number are required.' });
|
||||||
}
|
}
|
||||||
|
const MAX_IMAGE_BYTES = 512 * 1024; // 512 KB base64 limit
|
||||||
|
if (logo_data && Buffer.byteLength(logo_data, 'utf8') > MAX_IMAGE_BYTES) {
|
||||||
|
return res.status(400).json({ error: 'Logo image must be smaller than 512 KB.' });
|
||||||
|
}
|
||||||
|
|
||||||
db.prepare(`
|
db.prepare(`
|
||||||
UPDATE account SET
|
UPDATE account SET
|
||||||
@@ -121,15 +137,16 @@ app.put('/api/account/:id', requireAdmin, (req, res) => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// GET /api/account/:id — any authenticated user with access
|
// GET /api/account/:id — any authenticated user with access
|
||||||
|
// Routing/account numbers are only returned to admins (non-admins don't need them client-side)
|
||||||
app.get('/api/account/:id', (req, res) => {
|
app.get('/api/account/:id', (req, res) => {
|
||||||
if (!canAccessAccount(req.session, parseInt(req.params.id, 10))) {
|
if (!canAccessAccount(req.session, parseInt(req.params.id, 10))) {
|
||||||
return res.status(403).json({ error: 'Access denied.' });
|
return res.status(403).json({ error: 'Access denied.' });
|
||||||
}
|
}
|
||||||
const account = db.prepare(
|
const isAdmin = req.session.role === 'admin';
|
||||||
'SELECT id, bank_name, bank_info1, bank_info2, bank_info3, transit_code, ' +
|
const cols = isAdmin
|
||||||
'routing_number, account_number, current_check_no, ' +
|
? 'id, bank_name, bank_info1, bank_info2, bank_info3, transit_code, routing_number, account_number, current_check_no, company1, company2, company3, company4, check_position, second_signature'
|
||||||
'company1, company2, company3, company4, check_position, second_signature FROM account WHERE id = ?'
|
: 'id, bank_name, bank_info1, bank_info2, bank_info3, transit_code, current_check_no, company1, company2, company3, company4, check_position, second_signature';
|
||||||
).get(req.params.id);
|
const account = db.prepare(`SELECT ${cols} FROM account WHERE id = ?`).get(req.params.id);
|
||||||
if (!account) return res.status(404).json({ error: 'Account not found.' });
|
if (!account) return res.status(404).json({ error: 'Account not found.' });
|
||||||
res.json(account);
|
res.json(account);
|
||||||
});
|
});
|
||||||
|
|||||||
+54
-2
@@ -5,6 +5,45 @@ const router = express.Router();
|
|||||||
const bcrypt = require('bcryptjs');
|
const bcrypt = require('bcryptjs');
|
||||||
const db = require('../db/database');
|
const db = require('../db/database');
|
||||||
|
|
||||||
|
// ── Login rate limiter ────────────────────────────────────────────────────────
|
||||||
|
// Tracks failed login attempts per IP. After 10 failures within 15 minutes,
|
||||||
|
// further attempts are blocked until the window resets.
|
||||||
|
const loginAttempts = new Map(); // ip -> { count, resetAt }
|
||||||
|
const RATE_WINDOW_MS = 15 * 60 * 1000; // 15 minutes
|
||||||
|
const RATE_MAX_FAILS = 10;
|
||||||
|
|
||||||
|
function checkLoginRate(ip) {
|
||||||
|
const now = Date.now();
|
||||||
|
const entry = loginAttempts.get(ip);
|
||||||
|
if (!entry || now > entry.resetAt) {
|
||||||
|
loginAttempts.set(ip, { count: 0, resetAt: now + RATE_WINDOW_MS });
|
||||||
|
return true; // allow
|
||||||
|
}
|
||||||
|
return entry.count < RATE_MAX_FAILS;
|
||||||
|
}
|
||||||
|
|
||||||
|
function recordLoginFailure(ip) {
|
||||||
|
const now = Date.now();
|
||||||
|
const entry = loginAttempts.get(ip);
|
||||||
|
if (!entry || now > entry.resetAt) {
|
||||||
|
loginAttempts.set(ip, { count: 1, resetAt: now + RATE_WINDOW_MS });
|
||||||
|
} else {
|
||||||
|
entry.count++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function clearLoginFailures(ip) {
|
||||||
|
loginAttempts.delete(ip);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Purge stale entries every 30 minutes to prevent unbounded memory growth
|
||||||
|
setInterval(() => {
|
||||||
|
const now = Date.now();
|
||||||
|
for (const [ip, entry] of loginAttempts) {
|
||||||
|
if (now > entry.resetAt) loginAttempts.delete(ip);
|
||||||
|
}
|
||||||
|
}, 30 * 60 * 1000).unref();
|
||||||
|
|
||||||
// GET /api/auth/setup-needed — true when no users exist (first-run)
|
// GET /api/auth/setup-needed — true when no users exist (first-run)
|
||||||
router.get('/setup-needed', (req, res) => {
|
router.get('/setup-needed', (req, res) => {
|
||||||
const { n } = db.prepare('SELECT COUNT(*) AS n FROM users').get();
|
const { n } = db.prepare('SELECT COUNT(*) AS n FROM users').get();
|
||||||
@@ -34,15 +73,28 @@ router.post('/setup', async (req, res) => {
|
|||||||
|
|
||||||
// POST /api/auth/login
|
// POST /api/auth/login
|
||||||
router.post('/login', async (req, res) => {
|
router.post('/login', async (req, res) => {
|
||||||
|
const ip = req.ip || req.socket.remoteAddress || 'unknown';
|
||||||
|
|
||||||
|
if (!checkLoginRate(ip)) {
|
||||||
|
return res.status(429).json({ error: 'Too many failed login attempts. Please try again later.' });
|
||||||
|
}
|
||||||
|
|
||||||
const { username, password } = req.body;
|
const { username, password } = req.body;
|
||||||
if (!username || !password) return res.status(400).json({ error: 'Username and password required.' });
|
if (!username || !password) return res.status(400).json({ error: 'Username and password required.' });
|
||||||
|
|
||||||
const user = db.prepare('SELECT * FROM users WHERE username = ? COLLATE NOCASE').get(username.trim());
|
const user = db.prepare('SELECT * FROM users WHERE username = ? COLLATE NOCASE').get(username.trim());
|
||||||
if (!user) return res.status(401).json({ error: 'Invalid username or password.' });
|
if (!user) {
|
||||||
|
recordLoginFailure(ip);
|
||||||
|
return res.status(401).json({ error: 'Invalid username or password.' });
|
||||||
|
}
|
||||||
|
|
||||||
const match = await bcrypt.compare(password, user.password_hash);
|
const match = await bcrypt.compare(password, user.password_hash);
|
||||||
if (!match) return res.status(401).json({ error: 'Invalid username or password.' });
|
if (!match) {
|
||||||
|
recordLoginFailure(ip);
|
||||||
|
return res.status(401).json({ error: 'Invalid username or password.' });
|
||||||
|
}
|
||||||
|
|
||||||
|
clearLoginFailures(ip);
|
||||||
req.session.userId = user.id;
|
req.session.userId = user.id;
|
||||||
req.session.username = user.username;
|
req.session.username = user.username;
|
||||||
req.session.role = user.role;
|
req.session.role = user.role;
|
||||||
|
|||||||
+14
-2
@@ -54,6 +54,10 @@ router.post('/', (req, res) => {
|
|||||||
if (!account_id || !payee || !amount || !check_date) {
|
if (!account_id || !payee || !amount || !check_date) {
|
||||||
return res.status(400).json({ error: 'account_id, payee, amount, and check_date are required' });
|
return res.status(400).json({ error: 'account_id, payee, amount, and check_date are required' });
|
||||||
}
|
}
|
||||||
|
const parsedAmount = parseFloat(amount);
|
||||||
|
if (!isFinite(parsedAmount) || parsedAmount <= 0) {
|
||||||
|
return res.status(400).json({ error: 'Amount must be a positive number.' });
|
||||||
|
}
|
||||||
if (!isEditorForAccount(req.session, parseInt(account_id, 10))) {
|
if (!isEditorForAccount(req.session, parseInt(account_id, 10))) {
|
||||||
return res.status(403).json({ error: 'Write access required.' });
|
return res.status(403).json({ error: 'Write access required.' });
|
||||||
}
|
}
|
||||||
@@ -75,7 +79,7 @@ router.post('/', (req, res) => {
|
|||||||
|
|
||||||
const transaction = db.transaction(() => {
|
const transaction = db.transaction(() => {
|
||||||
const result = insertCheck.run(
|
const result = insertCheck.run(
|
||||||
account_id, checkNo, payee, parseFloat(amount), check_date,
|
account_id, checkNo, payee, parsedAmount, check_date,
|
||||||
memo || null, note1 || null, note2 || null,
|
memo || null, note1 || null, note2 || null,
|
||||||
payee_address1 || null, payee_address2 || null,
|
payee_address1 || null, payee_address2 || null,
|
||||||
payee_address3 || null, payee_address4 || null
|
payee_address3 || null, payee_address4 || null
|
||||||
@@ -99,6 +103,14 @@ router.put('/:id', (req, res) => {
|
|||||||
const { payee, amount, check_date, memo, note1, note2,
|
const { payee, amount, check_date, memo, note1, note2,
|
||||||
payee_address1, payee_address2, payee_address3, payee_address4 } = req.body;
|
payee_address1, payee_address2, payee_address3, payee_address4 } = req.body;
|
||||||
|
|
||||||
|
let parsedAmount = check.amount;
|
||||||
|
if (amount !== undefined) {
|
||||||
|
parsedAmount = parseFloat(amount);
|
||||||
|
if (!isFinite(parsedAmount) || parsedAmount <= 0) {
|
||||||
|
return res.status(400).json({ error: 'Amount must be a positive number.' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
db.prepare(`
|
db.prepare(`
|
||||||
UPDATE checks SET
|
UPDATE checks SET
|
||||||
payee = ?, amount = ?, check_date = ?, memo = ?, note1 = ?, note2 = ?,
|
payee = ?, amount = ?, check_date = ?, memo = ?, note1 = ?, note2 = ?,
|
||||||
@@ -106,7 +118,7 @@ router.put('/:id', (req, res) => {
|
|||||||
WHERE id = ?
|
WHERE id = ?
|
||||||
`).run(
|
`).run(
|
||||||
payee ?? check.payee,
|
payee ?? check.payee,
|
||||||
amount !== undefined ? parseFloat(amount) : check.amount,
|
parsedAmount,
|
||||||
check_date ?? check.check_date,
|
check_date ?? check.check_date,
|
||||||
memo ?? check.memo,
|
memo ?? check.memo,
|
||||||
note1 ?? check.note1,
|
note1 ?? check.note1,
|
||||||
|
|||||||
+18
-2
@@ -49,6 +49,14 @@ router.post('/', (req, res) => {
|
|||||||
if (!isEditorForAccount(req.session, parseInt(account_id, 10))) {
|
if (!isEditorForAccount(req.session, parseInt(account_id, 10))) {
|
||||||
return res.status(403).json({ error: 'Write access required.' });
|
return res.status(403).json({ error: 'Write access required.' });
|
||||||
}
|
}
|
||||||
|
if (Array.isArray(items)) {
|
||||||
|
for (const item of items) {
|
||||||
|
const a = parseFloat(item.amount);
|
||||||
|
if (!isFinite(a) || a <= 0) {
|
||||||
|
return res.status(400).json({ error: 'Each deposit item amount must be a positive number.' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const insert = db.transaction(() => {
|
const insert = db.transaction(() => {
|
||||||
const result = db.prepare(`
|
const result = db.prepare(`
|
||||||
@@ -76,7 +84,7 @@ router.post('/', (req, res) => {
|
|||||||
item.bank_no || null,
|
item.bank_no || null,
|
||||||
item.payee || null,
|
item.payee || null,
|
||||||
item.memo || null,
|
item.memo || null,
|
||||||
parseFloat(item.amount) || 0,
|
parseFloat(item.amount),
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -98,6 +106,14 @@ router.put('/:id', (req, res) => {
|
|||||||
|
|
||||||
const { deposit_date, currency, coin, cash_back, items } = req.body;
|
const { deposit_date, currency, coin, cash_back, items } = req.body;
|
||||||
if (!deposit_date) return res.status(400).json({ error: 'deposit_date is required.' });
|
if (!deposit_date) return res.status(400).json({ error: 'deposit_date is required.' });
|
||||||
|
if (Array.isArray(items)) {
|
||||||
|
for (const item of items) {
|
||||||
|
const a = parseFloat(item.amount);
|
||||||
|
if (!isFinite(a) || a <= 0) {
|
||||||
|
return res.status(400).json({ error: 'Each deposit item amount must be a positive number.' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const update = db.transaction(() => {
|
const update = db.transaction(() => {
|
||||||
db.prepare(`
|
db.prepare(`
|
||||||
@@ -124,7 +140,7 @@ router.put('/:id', (req, res) => {
|
|||||||
item.bank_no || null,
|
item.bank_no || null,
|
||||||
item.payee || null,
|
item.payee || null,
|
||||||
item.memo || null,
|
item.memo || null,
|
||||||
parseFloat(item.amount) || 0,
|
parseFloat(item.amount),
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -262,12 +262,20 @@ function confirmDeposits(db, records, account_id) {
|
|||||||
// POST /api/qbo-import/parse
|
// POST /api/qbo-import/parse
|
||||||
router.post('/parse', upload.single('file'), (req, res) => {
|
router.post('/parse', upload.single('file'), (req, res) => {
|
||||||
if (!req.file) return res.status(400).json({ error: 'No file uploaded.' });
|
if (!req.file) return res.status(400).json({ error: 'No file uploaded.' });
|
||||||
|
|
||||||
const type = req.body.type;
|
const type = req.body.type;
|
||||||
if (type !== 'checks' && type !== 'deposits') {
|
if (type !== 'checks' && type !== 'deposits') {
|
||||||
fs.unlink(req.file.path, () => {});
|
fs.unlink(req.file.path, () => {});
|
||||||
return res.status(400).json({ error: 'Invalid type. Must be "checks" or "deposits".' });
|
return res.status(400).json({ error: 'Invalid type. Must be "checks" or "deposits".' });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reject non-text MIME types — only CSV/plain text is expected
|
||||||
|
const mime = (req.file.mimetype || '').toLowerCase();
|
||||||
|
if (!mime.startsWith('text/') && mime !== 'application/csv' && mime !== 'application/vnd.ms-excel') {
|
||||||
|
fs.unlink(req.file.path, () => {});
|
||||||
|
return res.status(400).json({ error: 'File must be a CSV text file.' });
|
||||||
|
}
|
||||||
|
|
||||||
let text;
|
let text;
|
||||||
try {
|
try {
|
||||||
text = fs.readFileSync(req.file.path, 'utf8');
|
text = fs.readFileSync(req.file.path, 'utf8');
|
||||||
|
|||||||
Reference in New Issue
Block a user