580 lines
15 KiB
Markdown
580 lines
15 KiB
Markdown
# Security Fixes Implementation Guide
|
|
|
|
This document contains all the code changes required to fix the 3 critical security vulnerabilities identified in the audit report.
|
|
|
|
**Status**: Ready for implementation
|
|
**Date**: December 13, 2025
|
|
**Priority**: P0 (Critical) - Must be implemented before production
|
|
|
|
---
|
|
|
|
## Fix 1: Insecure Random Number Generation (CRITICAL)
|
|
|
|
**CVSS Score**: 8.1 (High)
|
|
**Location**: `backend/src/features/documents/api/documents.controller.ts`
|
|
**Lines**: 321-324
|
|
|
|
### Current Code (INSECURE):
|
|
```typescript
|
|
function cryptoRandom(): string {
|
|
// Safe unique suffix for object keys
|
|
return Math.random().toString(36).slice(2) + Date.now().toString(36);
|
|
}
|
|
```
|
|
|
|
### Required Changes:
|
|
|
|
#### Step 1: Add crypto import
|
|
**File**: `backend/src/features/documents/api/documents.controller.ts`
|
|
**Location**: Line 8 (after existing imports)
|
|
|
|
```typescript
|
|
import crypto from 'crypto';
|
|
```
|
|
|
|
#### Step 2: Replace cryptoRandom function
|
|
**File**: `backend/src/features/documents/api/documents.controller.ts`
|
|
**Location**: Lines 321-324
|
|
|
|
```typescript
|
|
function cryptoRandom(): string {
|
|
// Cryptographically secure random suffix for object keys
|
|
return crypto.randomBytes(32).toString('hex');
|
|
}
|
|
```
|
|
|
|
### Testing:
|
|
```bash
|
|
# Rebuild containers
|
|
make rebuild
|
|
|
|
# Verify document uploads still work
|
|
# Test that storage keys are now unpredictable
|
|
```
|
|
|
|
---
|
|
|
|
## Fix 2: Inadequate File Upload Validation (HIGH)
|
|
|
|
**CVSS Score**: 7.5 (High)
|
|
**Location**: `backend/src/features/documents/api/documents.controller.ts`
|
|
**Lines**: 205-216 and entire upload method
|
|
|
|
### Required Changes:
|
|
|
|
#### Step 1: Add file-type package
|
|
**File**: `backend/package.json`
|
|
**Location**: dependencies section
|
|
|
|
Add this line to dependencies:
|
|
```json
|
|
"file-type": "^19.8.0"
|
|
```
|
|
|
|
#### Step 2: Add imports
|
|
**File**: `backend/src/features/documents/api/documents.controller.ts`
|
|
**Location**: After line 7 (with other imports)
|
|
|
|
```typescript
|
|
import { fileTypeFromBuffer } from 'file-type';
|
|
import { Readable } from 'stream';
|
|
```
|
|
|
|
#### Step 3: Replace entire upload method
|
|
**File**: `backend/src/features/documents/api/documents.controller.ts`
|
|
**Location**: Lines 175-268 (entire `upload` method)
|
|
|
|
```typescript
|
|
async upload(request: FastifyRequest<{ Params: IdParams }>, reply: FastifyReply) {
|
|
const userId = (request as any).user?.sub as string;
|
|
const documentId = request.params.id;
|
|
|
|
logger.info('Document upload requested', {
|
|
operation: 'documents.upload',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
});
|
|
|
|
const doc = await this.service.getDocument(userId, documentId);
|
|
if (!doc) {
|
|
logger.warn('Document not found for upload', {
|
|
operation: 'documents.upload.not_found',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
});
|
|
return reply.code(404).send({ error: 'Not Found' });
|
|
}
|
|
|
|
const mp = await (request as any).file({ limits: { files: 1 } });
|
|
if (!mp) {
|
|
logger.warn('No file provided for upload', {
|
|
operation: 'documents.upload.no_file',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
});
|
|
return reply.code(400).send({ error: 'Bad Request', message: 'No file provided' });
|
|
}
|
|
|
|
// Define allowed MIME types and their corresponding magic byte signatures
|
|
const allowedTypes = new Map([
|
|
['application/pdf', new Set(['application/pdf'])],
|
|
['image/jpeg', new Set(['image/jpeg'])],
|
|
['image/png', new Set(['image/png'])],
|
|
]);
|
|
|
|
const contentType = mp.mimetype as string | undefined;
|
|
if (!contentType || !allowedTypes.has(contentType)) {
|
|
logger.warn('Unsupported file type for upload (header validation)', {
|
|
operation: 'documents.upload.unsupported_type',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
content_type: contentType,
|
|
file_name: mp.filename,
|
|
});
|
|
return reply.code(415).send({
|
|
error: 'Unsupported Media Type',
|
|
message: 'Only PDF, JPEG, and PNG files are allowed'
|
|
});
|
|
}
|
|
|
|
// Read first 4100 bytes to detect file type via magic bytes
|
|
const chunks: Buffer[] = [];
|
|
let totalBytes = 0;
|
|
const targetBytes = 4100;
|
|
|
|
for await (const chunk of mp.file) {
|
|
chunks.push(chunk);
|
|
totalBytes += chunk.length;
|
|
if (totalBytes >= targetBytes) {
|
|
break;
|
|
}
|
|
}
|
|
|
|
const headerBuffer = Buffer.concat(chunks);
|
|
|
|
// Validate actual file content using magic bytes
|
|
const detectedType = await fileTypeFromBuffer(headerBuffer);
|
|
|
|
if (!detectedType) {
|
|
logger.warn('Unable to detect file type from content', {
|
|
operation: 'documents.upload.type_detection_failed',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
content_type: contentType,
|
|
file_name: mp.filename,
|
|
});
|
|
return reply.code(415).send({
|
|
error: 'Unsupported Media Type',
|
|
message: 'Unable to verify file type from content'
|
|
});
|
|
}
|
|
|
|
// Verify detected type matches claimed Content-Type
|
|
const allowedDetectedTypes = allowedTypes.get(contentType);
|
|
if (!allowedDetectedTypes || !allowedDetectedTypes.has(detectedType.mime)) {
|
|
logger.warn('File content does not match Content-Type header', {
|
|
operation: 'documents.upload.type_mismatch',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
claimed_type: contentType,
|
|
detected_type: detectedType.mime,
|
|
file_name: mp.filename,
|
|
});
|
|
return reply.code(415).send({
|
|
error: 'Unsupported Media Type',
|
|
message: `File content (${detectedType.mime}) does not match claimed type (${contentType})`
|
|
});
|
|
}
|
|
|
|
const originalName: string = mp.filename || 'upload';
|
|
const ext = (() => {
|
|
const e = path.extname(originalName).replace(/^\./, '').toLowerCase();
|
|
if (e) return e;
|
|
if (contentType === 'application/pdf') return 'pdf';
|
|
if (contentType === 'image/jpeg') return 'jpg';
|
|
if (contentType === 'image/png') return 'png';
|
|
return 'bin';
|
|
})();
|
|
|
|
class CountingStream extends Transform {
|
|
public bytes = 0;
|
|
override _transform(chunk: any, _enc: BufferEncoding, cb: TransformCallback) {
|
|
this.bytes += chunk.length || 0;
|
|
cb(null, chunk);
|
|
}
|
|
}
|
|
|
|
const counter = new CountingStream();
|
|
|
|
// Create a new readable stream from the header buffer + remaining file chunks
|
|
const headerStream = Readable.from([headerBuffer]);
|
|
const remainingStream = mp.file;
|
|
|
|
// Pipe header first, then remaining content through counter
|
|
headerStream.pipe(counter, { end: false });
|
|
headerStream.on('end', () => {
|
|
remainingStream.pipe(counter);
|
|
});
|
|
|
|
const storage = getStorageService();
|
|
const bucket = 'documents';
|
|
const version = 'v1';
|
|
const unique = cryptoRandom();
|
|
const key = `documents/${userId}/${doc.vehicle_id}/${doc.id}/${version}/${unique}.${ext}`;
|
|
|
|
await storage.putObject(bucket, key, counter, contentType, { 'x-original-filename': originalName });
|
|
|
|
const updated = await this.service['repo'].updateStorageMeta(doc.id, userId, {
|
|
storage_bucket: bucket,
|
|
storage_key: key,
|
|
file_name: originalName,
|
|
content_type: contentType,
|
|
file_size: counter.bytes,
|
|
file_hash: null,
|
|
});
|
|
|
|
logger.info('Document upload completed', {
|
|
operation: 'documents.upload.success',
|
|
user_id: userId,
|
|
document_id: documentId,
|
|
vehicle_id: doc.vehicle_id,
|
|
file_name: originalName,
|
|
content_type: contentType,
|
|
detected_type: detectedType.mime,
|
|
file_size: counter.bytes,
|
|
storage_key: key,
|
|
});
|
|
|
|
return reply.code(200).send(updated);
|
|
}
|
|
```
|
|
|
|
### Testing:
|
|
```bash
|
|
# Install new dependency
|
|
cd backend && npm install
|
|
|
|
# Rebuild containers
|
|
make rebuild
|
|
|
|
# Test legitimate files
|
|
# - Upload PDF, JPEG, PNG - should succeed
|
|
|
|
# Test attack scenarios
|
|
# - Rename .exe to .pdf - should be REJECTED
|
|
# - Spoof Content-Type header - should be REJECTED
|
|
```
|
|
|
|
---
|
|
|
|
## Fix 3: Google Maps API Key Exposure (HIGH)
|
|
|
|
**CVSS Score**: 6.5 (Medium-High)
|
|
**Location**: Multiple files in `backend/src/features/stations/`
|
|
|
|
### Required Changes:
|
|
|
|
#### Backend Changes
|
|
|
|
##### Step 1: Update Google Maps Client
|
|
**File**: `backend/src/features/stations/external/google-maps/google-maps.client.ts`
|
|
|
|
**Change 1**: Update `transformPlaceToStation` method (lines 76-95)
|
|
```typescript
|
|
private transformPlaceToStation(place: GooglePlace, searchLat: number, searchLng: number): Station {
|
|
const distance = this.calculateDistance(
|
|
searchLat,
|
|
searchLng,
|
|
place.geometry.location.lat,
|
|
place.geometry.location.lng
|
|
);
|
|
|
|
// Store photo reference instead of full URL to avoid exposing API key
|
|
let photoReference: string | undefined;
|
|
if (place.photos && place.photos.length > 0 && place.photos[0]) {
|
|
photoReference = place.photos[0].photo_reference;
|
|
}
|
|
|
|
const station: Station = {
|
|
id: place.place_id,
|
|
placeId: place.place_id,
|
|
name: place.name,
|
|
address: place.vicinity,
|
|
latitude: place.geometry.location.lat,
|
|
longitude: place.geometry.location.lng,
|
|
distance
|
|
};
|
|
|
|
if (photoReference !== undefined) {
|
|
station.photoReference = photoReference;
|
|
}
|
|
|
|
if (place.opening_hours?.open_now !== undefined) {
|
|
station.isOpen = place.opening_hours.open_now;
|
|
}
|
|
|
|
if (place.rating !== undefined) {
|
|
station.rating = place.rating;
|
|
}
|
|
|
|
return station;
|
|
}
|
|
```
|
|
|
|
**Change 2**: Add new method after `transformPlaceToStation`
|
|
```typescript
|
|
/**
|
|
* Fetch photo from Google Maps API using photo reference
|
|
* Used by photo proxy endpoint to serve photos without exposing API key
|
|
*/
|
|
async fetchPhoto(photoReference: string, maxWidth: number = 400): Promise<Buffer> {
|
|
try {
|
|
const response = await axios.get(
|
|
`${this.baseURL}/photo`,
|
|
{
|
|
params: {
|
|
photo_reference: photoReference,
|
|
maxwidth: maxWidth,
|
|
key: this.apiKey
|
|
},
|
|
responseType: 'arraybuffer'
|
|
}
|
|
);
|
|
|
|
return Buffer.from(response.data);
|
|
} catch (error) {
|
|
logger.error('Failed to fetch photo from Google Maps', { error, photoReference });
|
|
throw error;
|
|
}
|
|
}
|
|
```
|
|
|
|
##### Step 2: Update Station Type (Backend)
|
|
**File**: `backend/src/features/stations/domain/stations.types.ts`
|
|
**Line**: 20
|
|
|
|
```typescript
|
|
// OLD:
|
|
photoUrl?: string;
|
|
|
|
// NEW:
|
|
photoReference?: string;
|
|
```
|
|
|
|
##### Step 3: Update Stations Controller
|
|
**File**: `backend/src/features/stations/api/stations.controller.ts`
|
|
|
|
**Add import** at the top:
|
|
```typescript
|
|
import { googleMapsClient } from '../external/google-maps/google-maps.client';
|
|
```
|
|
|
|
**Add method** after `removeSavedStation`:
|
|
```typescript
|
|
async getStationPhoto(request: FastifyRequest<{ Params: { reference: string } }>, reply: FastifyReply) {
|
|
try {
|
|
const { reference } = request.params;
|
|
|
|
if (!reference) {
|
|
return reply.code(400).send({
|
|
error: 'Bad Request',
|
|
message: 'Photo reference is required'
|
|
});
|
|
}
|
|
|
|
const photoBuffer = await googleMapsClient.fetchPhoto(reference);
|
|
|
|
return reply
|
|
.code(200)
|
|
.header('Content-Type', 'image/jpeg')
|
|
.header('Cache-Control', 'public, max-age=86400')
|
|
.send(photoBuffer);
|
|
} catch (error: any) {
|
|
logger.error('Error fetching station photo', {
|
|
error,
|
|
reference: request.params.reference
|
|
});
|
|
return reply.code(500).send({
|
|
error: 'Internal server error',
|
|
message: 'Failed to fetch station photo'
|
|
});
|
|
}
|
|
}
|
|
```
|
|
|
|
##### Step 4: Update Stations Routes
|
|
**File**: `backend/src/features/stations/api/stations.routes.ts`
|
|
|
|
**Add route** before the closing `};`:
|
|
```typescript
|
|
// GET /api/stations/photo/:reference - Proxy for Google Maps photos
|
|
fastify.get<{ Params: { reference: string } }>('/stations/photo/:reference', {
|
|
preHandler: [fastify.authenticate],
|
|
handler: stationsController.getStationPhoto.bind(stationsController)
|
|
});
|
|
```
|
|
|
|
##### Step 5: Update Stations Service
|
|
**File**: `backend/src/features/stations/domain/stations.service.ts`
|
|
**Line**: 144
|
|
|
|
```typescript
|
|
// OLD:
|
|
photoUrl: station?.photoUrl,
|
|
|
|
// NEW:
|
|
photoReference: station?.photoReference,
|
|
```
|
|
|
|
#### Frontend Changes
|
|
|
|
##### Step 6: Update Station Type (Frontend)
|
|
**File**: `frontend/src/features/stations/types/stations.types.ts`
|
|
**Line**: 60
|
|
|
|
```typescript
|
|
// OLD:
|
|
photoUrl?: string;
|
|
|
|
// NEW:
|
|
photoReference?: string;
|
|
```
|
|
|
|
##### Step 7: Create Photo Utils Helper
|
|
**File**: `frontend/src/features/stations/utils/photo-utils.ts` (NEW FILE)
|
|
|
|
```typescript
|
|
/**
|
|
* @ai-summary Helper utilities for station photos
|
|
*/
|
|
|
|
/**
|
|
* Generate secure photo URL using backend proxy endpoint
|
|
* This prevents exposing the Google Maps API key to the frontend
|
|
*/
|
|
export function getStationPhotoUrl(photoReference: string | undefined): string | undefined {
|
|
if (!photoReference) {
|
|
return undefined;
|
|
}
|
|
|
|
return `/api/stations/photo/${encodeURIComponent(photoReference)}`;
|
|
}
|
|
```
|
|
|
|
##### Step 8: Update StationCard Component
|
|
**File**: `frontend/src/features/stations/components/StationCard.tsx`
|
|
|
|
**Add import** at the top:
|
|
```typescript
|
|
import { getStationPhotoUrl } from '../utils/photo-utils';
|
|
```
|
|
|
|
**Update photo rendering** (lines 86-93):
|
|
```typescript
|
|
// OLD:
|
|
{station.photoUrl && (
|
|
<CardMedia
|
|
component="img"
|
|
height="200"
|
|
image={station.photoUrl}
|
|
alt={station.name}
|
|
sx={{ objectFit: 'cover' }}
|
|
/>
|
|
)}
|
|
|
|
// NEW:
|
|
{station.photoReference && (
|
|
<CardMedia
|
|
component="img"
|
|
height="200"
|
|
image={getStationPhotoUrl(station.photoReference)}
|
|
alt={station.name}
|
|
sx={{ objectFit: 'cover' }}
|
|
/>
|
|
)}
|
|
```
|
|
|
|
### Testing:
|
|
```bash
|
|
# Rebuild containers
|
|
make rebuild
|
|
|
|
# Test station search with photos
|
|
# Verify API key is NOT in network requests
|
|
# Verify photos still load correctly
|
|
# Test on mobile and desktop
|
|
```
|
|
|
|
---
|
|
|
|
## Implementation Checklist
|
|
|
|
- [ ] Apply Fix 1: Insecure Random Generation
|
|
- [ ] Add crypto import
|
|
- [ ] Replace cryptoRandom function
|
|
- [ ] Test document uploads
|
|
|
|
- [ ] Apply Fix 2: File Upload Validation
|
|
- [ ] Add file-type to package.json
|
|
- [ ] Run npm install
|
|
- [ ] Add imports
|
|
- [ ] Replace upload method
|
|
- [ ] Test with legitimate files
|
|
- [ ] Test with malicious files
|
|
|
|
- [ ] Apply Fix 3: API Key Exposure
|
|
- [ ] Update Google Maps client
|
|
- [ ] Update backend Station type
|
|
- [ ] Add controller method
|
|
- [ ] Add route
|
|
- [ ] Update service
|
|
- [ ] Update frontend Station type
|
|
- [ ] Create photo-utils.ts
|
|
- [ ] Update StationCard component
|
|
- [ ] Test photos load correctly
|
|
- [ ] Verify API key not in network
|
|
|
|
- [ ] Run all tests
|
|
- [ ] Backend tests: `cd backend && npm test`
|
|
- [ ] Frontend tests: `cd frontend && npm test`
|
|
|
|
- [ ] Rebuild and verify
|
|
- [ ] `make rebuild`
|
|
- [ ] Manual testing of all features
|
|
- [ ] Security verification (no API keys exposed)
|
|
|
|
- [ ] Create git commit with security fixes
|
|
|
|
---
|
|
|
|
## Security Verification
|
|
|
|
After implementing all fixes, verify:
|
|
|
|
1. **Document Storage Keys**: Now using `crypto.randomBytes()` - unpredictable
|
|
2. **File Upload**: Magic byte validation rejects spoofed files
|
|
3. **Google Maps API**: Key never sent to frontend, all requests proxied
|
|
|
|
Run security scan:
|
|
```bash
|
|
# Check for hardcoded secrets
|
|
grep -r "AIza" backend/ frontend/
|
|
# Should return no results
|
|
|
|
# Verify crypto usage
|
|
grep -r "Math.random()" backend/
|
|
# Should return no results in production code
|
|
```
|
|
|
|
---
|
|
|
|
## Post-Implementation
|
|
|
|
After all fixes are applied and tested:
|
|
|
|
1. Update AUDIT.md to reflect fixed status
|
|
2. Commit changes with security fix message
|
|
3. Run full test suite
|
|
4. Deploy to staging for verification
|
|
5. Schedule production deployment
|