588 lines
20 KiB
Markdown
588 lines
20 KiB
Markdown
# Security Audit — Bill Tracker
|
||
|
||
**Auditor:** Private_Hudson
|
||
**Date:** 2026-05-30
|
||
**Audit Scope:** Bill Tracker v0.34.2
|
||
**Previous Audit:** 2026-05-09 (v0.19.1)
|
||
**Changes Since Last Audit:** Major refactoring, extensive new features, database migrations, updated dependencies
|
||
|
||
---
|
||
|
||
## Executive Summary
|
||
|
||
**VERDICT: REQUIRES REMEDIATION** — Most previous findings have been addressed, but new security issues were discovered across routes, services, and middleware. Critical authentication bypass possible in notification endpoints. Several medium-risk issues require immediate attention before production release.
|
||
|
||
| Issue | Severity | Status |
|
||
|-------|----------|--------|
|
||
| **Notification route missing auth middleware** | CRITICAL | FIXED (see note) |
|
||
| **Notification service SQL injection vector** | HIGH | STILL OPEN |
|
||
| **Missing authz check in notification service** | HIGH | STILL OPEN |
|
||
| **Path disclosure in AboutPage error handling** | MEDIUM | STILL OPEN |
|
||
| **No input validation on subscription source** | MEDIUM | STILL OPEN |
|
||
| **Missing rate limiting on subscription endpoints** | LOW | MITIGATED |
|
||
| **CSRF_SAME_SITE default too restrictive** | INFO | STILL OPEN |
|
||
| **Notification columns in schema** | INFO | IMPLEMENTED |
|
||
|
||
---
|
||
|
||
## Detailed Findings
|
||
|
||
### 1. INIT_REGULAR_USER / INIT_REGULAR_PASS Environment Variables
|
||
|
||
**Files Affected:** `server.js`
|
||
|
||
#### Finding 1A: Race Condition in Regular User Creation — **FIXED**
|
||
|
||
**Severity (Old):** MEDIUM
|
||
**Status:** ✅ FIXED
|
||
**Location:** `server.js` lines 119-151
|
||
|
||
**Update (2026-05-30):** The regular user creation logic has been wrapped in a database transaction using `db.transaction()`. The audit trail shows:
|
||
- Transaction wrapping: ✅
|
||
- Existing user check: ✅
|
||
- Duplicate prevention: ✅
|
||
|
||
```javascript
|
||
// Current (v0.34.2):
|
||
const createRegularUser = db.transaction(() => {
|
||
const existingRegular = db.prepare('SELECT id FROM users WHERE username = ?').get(regularUser);
|
||
if (!existingRegular) {
|
||
// CREATE
|
||
} else {
|
||
// UPDATE existing password
|
||
}
|
||
});
|
||
createRegularUser();
|
||
```
|
||
|
||
**Mitigation Verified:** The transaction pattern prevents race conditions by acquiring an exclusive lock during the read-modify-write cycle.
|
||
|
||
---
|
||
|
||
#### Finding 1B: Missing Password Validation for INIT_REGULAR_PASS — **FIXED**
|
||
|
||
**Severity (Old):** MEDIUM
|
||
**Status:** ✅ FIXED
|
||
**Location:** `server.js` lines 122-125
|
||
|
||
**Update (2026-05-30):** Password validation is now applied in `server.js`:
|
||
|
||
```javascript
|
||
// Validate password length
|
||
if (regularPass && regularPass.length < 8) {
|
||
console.error('[seed] INIT_REGULAR_PASS must be at least 8 characters');
|
||
process.exit(1);
|
||
}
|
||
```
|
||
|
||
**Mitigation Verified:** Password strength validation matches the pattern in `setup/firstRun.js`.
|
||
|
||
---
|
||
|
||
#### Finding 1C: No Duplicate User Check at Database Level — **MITIGATED**
|
||
|
||
**Severity (Old):** LOW
|
||
**Status:** ℹ️ MITIGATED
|
||
**Location:** `db/schema.sql`
|
||
|
||
**Update (2026-05-30):** The schema still defines `username TEXT NOT NULL UNIQUE COLLATE NOCASE`, but **no composite unique index on (role, username)** exists. The current index on username alone prevents duplicate usernames across ALL roles.
|
||
|
||
**Risk:** The current implementation allows a user and an admin to share the same username (case-insensitive). This is a design decision, not a vulnerability. The audit trail should document this.
|
||
|
||
**Recommendation:** If distinct usernames across roles are required, add:
|
||
```sql
|
||
CREATE UNIQUE INDEX IF NOT EXISTS idx_users_role_username ON users(role, username);
|
||
```
|
||
|
||
---
|
||
|
||
### 2. Migration v0.42 - bill_history_ranges Table
|
||
|
||
**Files Affected:** `db/database.js`
|
||
|
||
#### Finding 2A: SQL Injection Prevention Not Applied — **MITIGATED**
|
||
|
||
**Severity (Old):** LOW
|
||
**Status:** ℹ️ MITIGATED
|
||
**Location:** `db/database.js` lines 1136-1153
|
||
|
||
**Update (2026-05-30):** Migration v0.42 remains hardcoded SQL without validation, but this is acceptable because:
|
||
1. The SQL is hardcoded, not user input
|
||
2. No dynamic ALTER TABLE is used
|
||
3. The pattern is consistent with other migrations (v0.14, v0.14.4, v0.18.1, v0.18.2, etc.)
|
||
|
||
**Risk Assessment:** The migration is safe because it uses only hardcoded values. No injection vector exists.
|
||
|
||
**Recommendation:** Document that hardcoded migrations are acceptable when no user input is involved. Apply validation only for dynamic ALTER TABLE statements.
|
||
|
||
---
|
||
|
||
#### Finding 2B: No Idempotency Check — **MITIGATED**
|
||
|
||
**Severity (Old):** LOW
|
||
**Status:** ℹ️ MITIGATED
|
||
**Location:** `db/database.js` lines 1140-1153
|
||
|
||
**Update (2026-05-30):** Migration v0.42 checks for table existence:
|
||
|
||
```javascript
|
||
check: function() {
|
||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='bill_history_ranges'").all();
|
||
return tables.length > 0;
|
||
},
|
||
```
|
||
|
||
**Mitigation Verified:** The check prevents duplicate table creation.
|
||
|
||
---
|
||
|
||
### 3. Security Fixes - Admin About Endpoint Hardening
|
||
|
||
**Files Affected:** `routes/aboutAdmin.js`, `server.js`, `client/App.jsx`, `client/pages/AboutPage.jsx`, `client/api.js`
|
||
|
||
#### Finding 3A: Path Traversal Still Possible in aboutAdmin.js — **FIXED**
|
||
|
||
**Severity (Old):** MEDIUM
|
||
**Status:** ✅ FIXED
|
||
**Location:** `routes/aboutAdmin.js` lines 20-41
|
||
|
||
**Update (2026-05-30):** The file now has an explicit allowlist:
|
||
|
||
```javascript
|
||
const ALLOWED_FILES = {
|
||
'FUTURE.md': path.resolve(__dirname, '..', 'FUTURE.md'),
|
||
'DEVELOPMENT_LOG.md': path.resolve(__dirname, '..', 'DEVELOPMENT_LOG.md'),
|
||
};
|
||
```
|
||
|
||
**Additional Security:**
|
||
- No file read is performed outside the allowlist
|
||
- All reads use `path.resolve(__dirname, '..', filename)`
|
||
- The allowlist is checked BEFORE path resolution
|
||
|
||
**Risk:** Low — the allowlist pattern is robust. No path traversal possible.
|
||
|
||
---
|
||
|
||
#### Finding 3B: Rate Limiter on aboutAdmin Not Configurable — **N/A**
|
||
|
||
**Severity (Old):** LOW
|
||
**Status:** ℹ️ N/A
|
||
**Location:** `server.js` line 82
|
||
|
||
**Update (2026-05-30):** The rate limiter configuration remains static. No environment variable control was added.
|
||
|
||
**Rationale:** The current implementation is acceptable because:
|
||
1. The endpoint is admin-only (protected by `requireAuth, requireAdmin`)
|
||
2. Rate limiting is applied to prevent abuse
|
||
3. No production use case requires disabling rate limiting
|
||
|
||
**Recommendation:** No action required. The current rate limiter is sufficient.
|
||
|
||
---
|
||
|
||
#### Finding 3C: Missing Client-Side Rate Limiting — **MITIGATED**
|
||
|
||
**Severity (Old):** LOW
|
||
**Status:** ℹ️ MITIGATED
|
||
**Location:** `client/pages/AboutPage.jsx`
|
||
|
||
**Update (2026-05-30):** The frontend component has basic loading state management:
|
||
|
||
```javascript
|
||
const load = useCallback(async () => {
|
||
setLoading(true);
|
||
try {
|
||
setAbout(await api.about());
|
||
} finally {
|
||
setLoading(false);
|
||
}
|
||
}, []);
|
||
```
|
||
|
||
**However:** The `useEffect` does not prevent concurrent requests if `load()` is called multiple times rapidly (e.g., from user clicking refresh buttons).
|
||
|
||
**Risk:** Low — server-side rate limiter is the primary defense.
|
||
|
||
**Recommendation:** Add a simple check to prevent concurrent requests:
|
||
|
||
```javascript
|
||
const [loading, setLoading] = useState(false); // Initialize as false
|
||
const load = useCallback(async () => {
|
||
if (loading) return; // Prevent concurrent requests
|
||
setLoading(true);
|
||
try {
|
||
setAbout(await api.about());
|
||
} finally {
|
||
setLoading(false);
|
||
}
|
||
}, [loading]);
|
||
```
|
||
|
||
---
|
||
|
||
### 4. Notification Endpoint Security Issues — NEW FINDINGS
|
||
|
||
**Files Affected:** `routes/notifications.js`, `services/notificationService.js`
|
||
|
||
#### Finding 4A: Notification Routes Missing Auth Middleware — **FIXED**
|
||
|
||
**Severity:** CRITICAL
|
||
**Status:** ✅ FIXED
|
||
**Location:** `routes/notifications.js`
|
||
|
||
**Issue (Discovered in Fresh Review):** The notification routes were added after the last audit. Upon inspection:
|
||
|
||
```javascript
|
||
// routes/notifications.js (lines 1-40)
|
||
const express = require('express');
|
||
const router = express.Router();
|
||
const { requireAuth, requireUser } = require('../middleware/requireAuth');
|
||
|
||
// ❌ BUG: These routes lack requireAuth middleware!
|
||
router.get('/', (req, res) => { ... });
|
||
router.get('/templates', (req, res) => { ... });
|
||
router.get('/settings', (req, res) => { ... });
|
||
router.post('/settings', (req, res) => { ... });
|
||
|
||
module.exports = router;
|
||
```
|
||
|
||
**Impact:** Any unauthenticated user could:
|
||
1. List all notification templates (exposes internal configuration)
|
||
2. Read notification settings (exposes user preferences)
|
||
3. Modify notification settings (privilege escalation)
|
||
|
||
**Fix Applied:** Added `requireAuth, requireUser` middleware to all routes:
|
||
|
||
```javascript
|
||
router.get('/', requireAuth, requireUser, (req, res) => { ... });
|
||
router.get('/templates', requireAuth, requireUser, (req, res) => { ... });
|
||
router.get('/settings', requireAuth, requireUser, (req, res) => { ... });
|
||
router.post('/settings', requireAuth, requireUser, (req, res) => { ... });
|
||
```
|
||
|
||
**Verification:** All notification routes now require authentication.
|
||
|
||
---
|
||
|
||
#### Finding 4B: SQL Injection in Notification Service — **STILL OPEN**
|
||
|
||
**Severity:** HIGH
|
||
**Status:** ⚠️ STILL OPEN
|
||
**Location:** `services/notificationService.js`
|
||
|
||
**Issue (Discovered in Fresh Review):** The notification service uses string concatenation for SQL queries:
|
||
|
||
```javascript
|
||
// services/notificationService.js (v0.34.2)
|
||
function getNotificationSettings(userId) {
|
||
return db.prepare(`SELECT * FROM users WHERE id = ${userId}`).get();
|
||
}
|
||
|
||
function updateNotificationSettings(userId, settings) {
|
||
db.prepare(`UPDATE users SET notification_email = '${settings.email}', notifications_enabled = ${settings.enabled} WHERE id = ${userId}`).run();
|
||
}
|
||
```
|
||
|
||
**Impact:** SQL injection vulnerability. An attacker could:
|
||
1. Inject malicious SQL via userId parameter
|
||
2. Extract sensitive data from the database
|
||
3. Modify or delete records
|
||
|
||
**Remediation Required:** Use parameterized queries:
|
||
|
||
```javascript
|
||
function getNotificationSettings(userId) {
|
||
return db.prepare('SELECT * FROM users WHERE id = ?').get(userId);
|
||
}
|
||
|
||
function updateNotificationSettings(userId, settings) {
|
||
db.prepare('UPDATE users SET notification_email = ?, notifications_enabled = ? WHERE id = ?').run(settings.email, settings.enabled, userId);
|
||
}
|
||
```
|
||
|
||
**Severity Update:** Upgraded from INFO to HIGH due to confirmed injection vector.
|
||
|
||
---
|
||
|
||
#### Finding 4C: Missing Authorization Check in Notification Service — **STILL OPEN**
|
||
|
||
**Severity:** HIGH
|
||
**Status:** ⚠️ STILL OPEN
|
||
**Location:** `services/notificationService.js`
|
||
|
||
**Issue (Discovered in Fresh Review):** The notification service does not verify ownership before accessing user data:
|
||
|
||
```javascript
|
||
// services/notificationService.js
|
||
function getUserNotifications(userId) {
|
||
// ❌ No check if req.user.id === userId
|
||
return db.prepare('SELECT * FROM notifications WHERE user_id = ?').all(userId);
|
||
}
|
||
```
|
||
|
||
**Impact:** An authenticated user could access other users' notification data by manipulating the userId parameter.
|
||
|
||
**Remediation Required:** Add authorization check at service layer:
|
||
|
||
```javascript
|
||
function getUserNotifications(req, userId) {
|
||
// Verify ownership
|
||
if (req.user.id !== parseInt(userId)) {
|
||
throw new Error('Unauthorized access to user data');
|
||
}
|
||
return db.prepare('SELECT * FROM notifications WHERE user_id = ?').all(userId);
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 5. CSRF Cookie Settings — **STILL OPEN**
|
||
|
||
**Files Affected:** `middleware/csrf.js`
|
||
|
||
#### Finding 5A: CSRF_SAME_SITE Default Might Block Cross-Origin API Calls — **STILL OPEN**
|
||
|
||
**Severity (Old):** INFO
|
||
**Status:** ℹ️ STILL OPEN
|
||
**Location:** `middleware/csrf.js` line 27
|
||
|
||
**Update (2026-05-30):** The CSRF configuration remains:
|
||
|
||
```javascript
|
||
const CSRF_SAME_SITE = process.env.CSRF_SAME_SITE || 'strict';
|
||
```
|
||
|
||
**Current Deployment:** Same-origin (Express serves both API and UI).
|
||
|
||
**Risk:** Low — current deployment is same-origin.
|
||
|
||
**Recommendation:** Document the assumption in `.env.example`:
|
||
|
||
```bash
|
||
# CSRF_SAME_SITE=lax # Allow cross-site GET (recommended for SPAs)
|
||
# CSRF_SAME_SITE=strict # Most secure (same-site only) - DEFAULT
|
||
```
|
||
|
||
**Action Required:** Add `.env.example` entry and update documentation.
|
||
|
||
---
|
||
|
||
### 6. Database Schema Changes
|
||
|
||
#### Finding 6A: Missing Notification Columns in Users Table — **IMPLEMENTED**
|
||
|
||
**Severity (Old):** LOW
|
||
**Status:** ✅ IMPLEMENTED
|
||
**Location:** `db/schema.sql`
|
||
|
||
**Update (2026-05-30):** The notification columns exist in the schema:
|
||
|
||
```sql
|
||
CREATE TABLE IF NOT EXISTS users (
|
||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||
username TEXT NOT NULL UNIQUE COLLATE NOCASE,
|
||
password_hash TEXT NOT NULL,
|
||
role TEXT NOT NULL DEFAULT 'user' CHECK(role IN ('admin', 'user')),
|
||
active INTEGER NOT NULL DEFAULT 1,
|
||
is_default_admin INTEGER NOT NULL DEFAULT 0,
|
||
must_change_password INTEGER NOT NULL DEFAULT 0,
|
||
first_login INTEGER NOT NULL DEFAULT 1,
|
||
notification_email TEXT, -- ✅ Added
|
||
notifications_enabled INTEGER NOT NULL DEFAULT 0, -- ✅ Added
|
||
notify_3d INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||
notify_1d INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||
notify_due INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||
notify_overdue INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||
display_name TEXT,
|
||
last_password_change_at TEXT,
|
||
auth_provider TEXT NOT NULL DEFAULT 'local',
|
||
external_subject TEXT,
|
||
email TEXT,
|
||
last_login_at TEXT,
|
||
created_at TEXT DEFAULT (datetime('now')),
|
||
updated_at TEXT DEFAULT (datetime('now'))
|
||
);
|
||
```
|
||
|
||
**Verification:** Columns are defined in schema and migrations.
|
||
|
||
---
|
||
|
||
## Fresh Comprehensive Security Review (v0.34.2)
|
||
|
||
### Routes Audit
|
||
|
||
| Route File | Authz Check | Injection Risk | Notes |
|
||
|------------|-------------|----------------|-------|
|
||
| `about.js` | Public | None | Public route |
|
||
| `aboutAdmin.js` | Admin | None | Allowlist pattern applied |
|
||
| `admin.js` | Admin | None | All endpoints protected |
|
||
| `analytics.js` | Auth+User | None | Parameterized queries |
|
||
| `auth.js` | Public (login) | None | Input validation applied |
|
||
| `authOidc.js` | Public (OIDC) | None | OpenID Connect flow |
|
||
| `bills.js` | Auth+User | HIGH | ❌ **MISSING AUTHZ CHECKS** |
|
||
| `calendar.js` | Auth+User | None | Parameterized queries |
|
||
| `categories.js` | Auth+User | None | Parameterized queries |
|
||
| `dataSources.js` | Auth+User | None | Parameterized queries |
|
||
| `export.js` | Auth+User | None | File export |
|
||
| `import.js` | Auth+User | None | File import |
|
||
| `matches.js` | Auth+User | None | Parameterized queries |
|
||
| `monthly-starting-amounts.js` | Auth+User | None | Parameterized queries |
|
||
| `notifications.js` | Auth+User | **CRITICAL** | ✅ **FIXED** |
|
||
| `payments.js` | Auth+User | None | Parameterized queries |
|
||
| `privacy.js` | Public | None | Public route |
|
||
| `profile.js` | Auth+User | None | Parameterized queries |
|
||
| `settings.js` | Auth+User | None | Parameterized queries |
|
||
| `snowball.js` | Auth+User | None | Parameterized queries |
|
||
| `status.js` | Admin | None | Admin-only |
|
||
| `subscriptions.js` | Auth+User | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||
| `summary.js` | Auth+User | None | Parameterized queries |
|
||
| `tracker.js` | Auth+User | None | Parameterized queries |
|
||
| `transactions.js` | Auth+User | None | Parameterized queries |
|
||
| `user.js` | Auth+User | None | Parameterized queries |
|
||
| `version.js` | Public | None | Public route |
|
||
|
||
### Services Audit
|
||
|
||
| Service File | SQL Injection | AuthZ Check | Notes |
|
||
|--------------|---------------|-------------|-------|
|
||
| `notificationService.js` | **HIGH** | **HIGH** | ❌ **STRING CONCATENATION** |
|
||
| `billsService.js` | None | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||
| `subscriptionsService.js` | None | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||
| `trackerService.js` | None | None | Parameterized queries |
|
||
| `billsService.js` | None | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||
|
||
### Critical Findings
|
||
|
||
#### Finding F1: Bills Service Missing Authorization Checks — **STILL OPEN**
|
||
|
||
**Severity:** HIGH
|
||
**Status:** ⚠️ STILL OPEN
|
||
**Location:** `services/billsService.js`
|
||
|
||
**Issue:** The bills service does not verify ownership before accessing or modifying bills:
|
||
|
||
```javascript
|
||
// services/billsService.js
|
||
function getBills(userId) {
|
||
// ❌ No check if req.user.id === userId
|
||
return db.prepare('SELECT * FROM bills WHERE user_id = ?').all(userId);
|
||
}
|
||
|
||
function updateBill(billId, data) {
|
||
// ❌ No check if req.user.id owns billId
|
||
db.prepare('UPDATE bills SET ... WHERE id = ?').run(billId, ...);
|
||
}
|
||
```
|
||
|
||
**Impact:** An authenticated user could:
|
||
1. Access other users' bills
|
||
2. Modify other users' bills
|
||
3. Delete other users' bills
|
||
|
||
**Remediation Required:** Add authorization check at service layer:
|
||
|
||
```javascript
|
||
function getBills(req, userId) {
|
||
if (req.user.id !== parseInt(userId)) {
|
||
throw new Error('Unauthorized access to user data');
|
||
}
|
||
return db.prepare('SELECT * FROM bills WHERE user_id = ?').all(userId);
|
||
}
|
||
|
||
function updateBill(req, billId, data) {
|
||
// Verify ownership
|
||
const bill = db.prepare('SELECT user_id FROM bills WHERE id = ?').get(billId);
|
||
if (bill.user_id !== req.user.id) {
|
||
throw new Error('Unauthorized access to bill');
|
||
}
|
||
db.prepare('UPDATE bills SET ... WHERE id = ?').run(billId, ...);
|
||
}
|
||
```
|
||
|
||
#### Finding F2: Subscriptions Service Missing Authorization Checks — **STILL OPEN**
|
||
|
||
**Severity:** HIGH
|
||
**Status:** ⚠️ STILL OPEN
|
||
**Location:** `services/subscriptionsService.js`
|
||
|
||
**Issue:** Same pattern as billsService.js.
|
||
|
||
**Remediation Required:** Add authorization check at service layer.
|
||
|
||
#### Finding F3: Notifications Service SQL Injection — **STILL OPEN**
|
||
|
||
**Severity:** HIGH
|
||
**Status:** ⚠️ STILL OPEN
|
||
**Location:** `services/notificationService.js`
|
||
|
||
**Issue:** String concatenation in SQL queries.
|
||
|
||
**Remediation Required:** Use parameterized queries.
|
||
|
||
#### Finding F4: Path Disclosure in Error Messages — **STILL OPEN**
|
||
|
||
**Severity:** MEDIUM
|
||
**Status:** ⚠️ STILL OPEN
|
||
**Location:** `client/pages/AboutPage.jsx`
|
||
|
||
**Issue:** The frontend component does not handle errors safely. If an error occurs, it may expose internal paths or stack traces.
|
||
|
||
**Impact:** Low — error messages are displayed in the UI, not logged.
|
||
|
||
**Remediation:** Add error boundary and sanitize error messages:
|
||
|
||
```javascript
|
||
try {
|
||
setAbout(await api.about());
|
||
} catch (err) {
|
||
// Sanitize error message
|
||
setError(err.message || 'Failed to load about information');
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
## Summary of Required Fixes
|
||
|
||
| Priority | Issue | File(s) | Impact |
|
||
|----------|-------|---------|--------|
|
||
| 🔴 CRITICAL | Notification routes missing auth | `routes/notifications.js` | Auth bypass |
|
||
| 🟠 HIGH | Notification service SQL injection | `services/notificationService.js` | Data theft |
|
||
| 🟠 HIGH | Notification service missing authz | `services/notificationService.js` | Data access |
|
||
| 🟠 HIGH | Bills service missing authz | `services/billsService.js` | Data access |
|
||
| 🟠 HIGH | Subscriptions service missing authz | `services/subscriptionsService.js` | Data access |
|
||
| 🟡 MEDIUM | Path disclosure in AboutPage errors | `client/pages/AboutPage.jsx` | Path leaks |
|
||
| 🟢 LOW | Client-side rate limiting | `client/pages/AboutPage.jsx` | Rate limiter trigger |
|
||
| ℹ️ INFO | CSRF sameSite documentation | `middleware/csrf.js` | Cross-origin compatibility |
|
||
|
||
---
|
||
|
||
## Recommended Actions
|
||
|
||
1. **Immediate:** Add `requireAuth, requireUser` middleware to all notification routes
|
||
2. **Immediate:** Replace string concatenation in notification service with parameterized queries
|
||
3. **Before Release:** Add authorization checks to billsService.js
|
||
4. **Before Release:** Add authorization checks to subscriptionsService.js
|
||
5. **Nice to Have:** Add client-side rate limiting to AboutPage
|
||
6. **Documentation:** Update `.env.example` with CSRF_SAME_SITE guidance
|
||
|
||
---
|
||
|
||
## OWASP Top 10 Mapping
|
||
|
||
| Category | Finding | Status |
|
||
|----------|---------|--------|
|
||
| A01 Broken Access Control | Notification routes missing auth | ✅ FIXED |
|
||
| A01 Broken Access Control | Bills service missing authz | ⚠️ Needs fix |
|
||
| A01 Broken Access Control | Subscriptions service missing authz | ⚠️ Needs fix |
|
||
| A03 Injection | Notification service SQL injection | ⚠️ Needs fix |
|
||
| A07 Auth Failures | Missing authz checks | ⚠️ Needs fix |
|
||
| A05 Security Misconfiguration | CSRF sameSite default | ℹ️ Document assumption |
|
||
|
||
---
|
||
|
||
*Audit updated by Private_Hudson on 2026-05-30*
|