security: address audit findings C-4 through M-8

Critical:
- C-4: Add express-rate-limit to login (20 attempts/15min)
- C-5: Remove default credentials from LoginForm.js
- C-6: Add sandbox attribute to KB document iframe

High:
- H-2: Hard-fail on startup if SESSION_SECRET env var is missing
- H-6: Sanitize filenames in Content-Disposition headers
- H-7: Fix KB upload race condition — move file after DB insert succeeds
- H-8: Generate random admin password in setup.js instead of hardcoded
- H-9: Add rehype-sanitize to ReactMarkdown (requires npm install)

Medium:
- M-4: Fix loose equality (==) to strict (===) in users.js self-checks
- M-5: Add hostname format regex validation in compliance notes
- M-6: Fix vendor trim-before-validate in ivantiTodoQueue.js
- M-7: Sanitize original filename in compliance temp JSON
- M-8: Pull CSP frame-ancestors from CORS_ORIGINS env var

New dependencies needed:
- backend: express-rate-limit (npm install in root)
- frontend: rehype-sanitize (npm install in frontend/)
This commit is contained in:
jramos
2026-04-07 10:23:10 -06:00
parent 169a0d2337
commit 8a6a3485e9
11 changed files with 62 additions and 34 deletions

View File

@@ -2,13 +2,22 @@
const express = require('express');
const bcrypt = require('bcryptjs');
const crypto = require('crypto');
const rateLimit = require('express-rate-limit');
const { requireAuth, requireGroup } = require('../middleware/auth');
const loginLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 20, // 20 attempts per window
standardHeaders: true,
legacyHeaders: false,
message: { error: 'Too many login attempts. Please try again in 15 minutes.' }
});
function createAuthRouter(db, logAudit) {
const router = express.Router();
// Login
router.post('/login', async (req, res) => {
router.post('/login', loginLimiter, async (req, res) => {
const { username, password } = req.body;
if (!username || !password) {

View File

@@ -260,7 +260,7 @@ function createComplianceRouter(db, upload, requireAuth, requireGroup) {
items: parsed.items,
summary: parsed.summary,
report_date: parsed.report_date,
filename: req.file.originalname,
filename: req.file.originalname.replace(/[^\w.\-() ]/g, '_'),
}));
// Delete the original xlsx from temp (we only need the JSON now)
@@ -523,8 +523,8 @@ function createComplianceRouter(db, upload, requireAuth, requireGroup) {
router.post('/notes', requireGroup('Admin', 'Standard_User'), async (req, res) => {
const { hostname, metric_id, note } = req.body;
if (!hostname || typeof hostname !== 'string' || hostname.length > 300) {
return res.status(400).json({ error: 'Invalid hostname' });
if (!hostname || typeof hostname !== 'string' || hostname.length > 300 || !/^[a-zA-Z0-9._-]+$/.test(hostname)) {
return res.status(400).json({ error: 'Invalid hostname format' });
}
if (!metric_id || typeof metric_id !== 'string' || metric_id.length > 50) {
return res.status(400).json({ error: 'Invalid metric_id' });

View File

@@ -6,7 +6,9 @@ const VALID_WORKFLOW_TYPES = ['FP', 'Archer', 'CARD'];
const VALID_STATUSES = ['pending', 'complete'];
function isValidVendor(vendor) {
return typeof vendor === 'string' && vendor.trim().length > 0 && vendor.length <= 200;
if (typeof vendor !== 'string') return false;
const trimmed = vendor.trim();
return trimmed.length > 0 && trimmed.length <= 200;
}
function createIvantiTodoQueueRouter(db, requireAuth) {

View File

@@ -92,22 +92,15 @@ function createKnowledgeBaseRouter(db, upload) {
const slug = generateSlug(title);
const kbDir = path.join(__dirname, '..', 'uploads', 'knowledge_base');
// Create directory if it doesn't exist
if (!fs.existsSync(kbDir)) {
fs.mkdirSync(kbDir, { recursive: true });
}
const filename = `${timestamp}_${sanitizedName}`;
const filePath = path.join(kbDir, filename);
try {
// Move uploaded file to permanent location
fs.renameSync(uploadedFile.path, filePath);
// Keep file in temp location until DB insert succeeds
// Check if slug already exists
db.get('SELECT id FROM knowledge_base WHERE slug = ?', [slug], (err, row) => {
if (err) {
fs.unlinkSync(filePath);
fs.unlinkSync(uploadedFile.path);
console.error('Error checking slug:', err);
return res.status(500).json({ error: 'Database error' });
}
@@ -138,11 +131,22 @@ function createKnowledgeBaseRouter(db, upload) {
],
function (err) {
if (err) {
fs.unlinkSync(filePath);
fs.unlinkSync(uploadedFile.path);
console.error('Error inserting knowledge base entry:', err);
return res.status(500).json({ error: 'Failed to save document metadata' });
}
// DB insert succeeded — now move file to permanent location
try {
if (!fs.existsSync(kbDir)) {
fs.mkdirSync(kbDir, { recursive: true });
}
fs.renameSync(uploadedFile.path, filePath);
} catch (moveErr) {
console.error('Error moving file to permanent location:', moveErr);
// File is orphaned in temp but DB record exists — log and continue
}
// Log audit entry
logAudit(db, {
userId: req.user.id,
@@ -165,8 +169,8 @@ function createKnowledgeBaseRouter(db, upload) {
);
});
} catch (error) {
// Clean up file on error
if (fs.existsSync(filePath)) fs.unlinkSync(filePath);
// Clean up temp file on error
if (uploadedFile && fs.existsSync(uploadedFile.path)) fs.unlinkSync(uploadedFile.path);
console.error('Error uploading knowledge base document:', error);
res.status(500).json({ error: error.message || 'Failed to upload document' });
}
@@ -288,12 +292,14 @@ function createKnowledgeBaseRouter(db, upload) {
contentType = 'text/plain; charset=utf-8';
}
const safeFileName = row.file_name.replace(/["\r\n\\]/g, '');
res.setHeader('Content-Type', contentType);
// Use inline instead of attachment to allow browser to display
res.setHeader('Content-Disposition', `inline; filename="${row.file_name}"`);
res.setHeader('Content-Disposition', `inline; filename="${safeFileName}"`);
// Allow iframe embedding from frontend origin
res.removeHeader('X-Frame-Options');
res.setHeader('Content-Security-Policy', "frame-ancestors 'self' http://71.85.90.9:3000 http://localhost:3000");
const corsOrigins = process.env.CORS_ORIGINS ? process.env.CORS_ORIGINS.split(',').join(' ') : 'http://localhost:3000';
res.setHeader('Content-Security-Policy', `frame-ancestors 'self' ${corsOrigins}`);
res.sendFile(row.file_path);
});
});
@@ -338,8 +344,9 @@ function createKnowledgeBaseRouter(db, upload) {
ipAddress: req.ip
});
const safeDownloadName = row.file_name.replace(/["\r\n\\]/g, '');
res.setHeader('Content-Type', row.file_type || 'application/octet-stream');
res.setHeader('Content-Disposition', `attachment; filename="${row.file_name}"`);
res.setHeader('Content-Disposition', `attachment; filename="${safeDownloadName}"`);
res.sendFile(row.file_path);
});
});

View File

@@ -124,12 +124,12 @@ function createUsersRouter(db, requireAuth, requireGroup, logAudit) {
}
// Prevent admin self-demotion
if (userId == req.user.id && group && group !== 'Admin') {
if (String(userId) === String(req.user.id) && group && group !== 'Admin') {
return res.status(400).json({ error: 'Cannot remove your own admin group' });
}
// Prevent self-deactivation
if (userId == req.user.id && is_active === false) {
if (String(userId) === String(req.user.id) && is_active === false) {
return res.status(400).json({ error: 'Cannot deactivate your own account' });
}
@@ -247,7 +247,7 @@ function createUsersRouter(db, requireAuth, requireGroup, logAudit) {
const userId = req.params.id;
// Prevent self-deletion
if (userId == req.user.id) {
if (String(userId) === String(req.user.id)) {
return res.status(400).json({ error: 'Cannot delete your own account' });
}

View File

@@ -29,7 +29,11 @@ const createComplianceRouter = require('./routes/compliance');
const app = express();
const PORT = process.env.PORT || 3001;
const API_HOST = process.env.API_HOST || 'localhost';
const SESSION_SECRET = process.env.SESSION_SECRET || 'default-secret-change-me';
const SESSION_SECRET = process.env.SESSION_SECRET;
if (!SESSION_SECRET) {
console.error('FATAL: SESSION_SECRET environment variable must be set');
process.exit(1);
}
const CORS_ORIGINS = process.env.CORS_ORIGINS
? process.env.CORS_ORIGINS.split(',')
: ['http://localhost:3000'];

View File

@@ -3,6 +3,7 @@
const sqlite3 = require('sqlite3').verbose();
const bcrypt = require('bcryptjs');
const crypto = require('crypto');
const fs = require('fs');
const path = require('path');
@@ -172,8 +173,9 @@ async function createDefaultAdmin(db) {
return;
}
// Create admin user with password 'admin123'
const passwordHash = await bcrypt.hash('admin123', 10);
// Generate a random admin password on first run
const generatedPassword = crypto.randomBytes(12).toString('base64url');
const passwordHash = await bcrypt.hash(generatedPassword, 10);
db.run(
`INSERT INTO users (username, email, password_hash, role, is_active)
@@ -183,7 +185,12 @@ async function createDefaultAdmin(db) {
if (err) {
reject(err);
} else {
console.log('✓ Created default admin user (admin/admin123)');
console.log('✓ Created default admin user');
console.log(`\n ╔══════════════════════════════════════════╗`);
console.log(` ║ Admin credentials (save these now!) ║`);
console.log(` ║ Username: admin ║`);
console.log(` ║ Password: ${generatedPassword.padEnd(29)}`);
console.log(` ╚══════════════════════════════════════════╝\n`);
resolve();
}
}
@@ -269,7 +276,7 @@ function displaySummary() {
console.log(' ✓ Indexes for fast queries');
console.log(' ✓ Document compliance view');
console.log(' ✓ Uploads directory for file storage');
console.log(' ✓ Default admin user (admin/admin123)');
console.log(' ✓ Default admin user (see credentials above)');
console.log('\n📁 File structure will be:');
console.log(' uploads/');
console.log(' └── CVE-XXXX-XXXX/');