Security fix: Implement magic byte validation for file uploads (Fix 2)
Fixed HIGH severity security vulnerability (CVSS 7.5) where file upload validation relied solely on Content-Type headers, allowing malicious files with spoofed MIME types to bypass validation. Changes: - Updated file-type dependency to v16.5.4 (last CommonJS version) - Added magic byte (file signature) validation using fileTypeFromBuffer - Read first 4100 bytes of upload to detect actual file type - Verify detected type matches claimed Content-Type header - Reject files where content doesn't match headers - Enhanced logging with detected_type for audit trail Security impact: - Prevents .exe files renamed to .pdf from being uploaded - Prevents Content-Type header spoofing attacks - Validates file content at binary level, not just metadata Status: Fix 2 complete - Fix 1: crypto.randomBytes() ✓ - Fix 2: Magic byte validation ✓ - Fix 3: Google Maps API proxy ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -34,7 +34,7 @@
|
|||||||
"fastify-plugin": "^4.5.1",
|
"fastify-plugin": "^4.5.1",
|
||||||
"@fastify/autoload": "^5.8.0",
|
"@fastify/autoload": "^5.8.0",
|
||||||
"get-jwks": "^9.0.0",
|
"get-jwks": "^9.0.0",
|
||||||
"file-type": "^19.8.0"
|
"file-type": "^16.5.4"
|
||||||
},
|
},
|
||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
"@types/node": "^20.10.0",
|
"@types/node": "^20.10.0",
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ import { logger } from '../../../core/logging/logger';
|
|||||||
import path from 'path';
|
import path from 'path';
|
||||||
import { Transform, TransformCallback } from 'stream';
|
import { Transform, TransformCallback } from 'stream';
|
||||||
import crypto from 'crypto';
|
import crypto from 'crypto';
|
||||||
|
import FileType from 'file-type';
|
||||||
|
import { Readable } from 'stream';
|
||||||
|
|
||||||
export class DocumentsController {
|
export class DocumentsController {
|
||||||
private readonly service = new DocumentsService();
|
private readonly service = new DocumentsService();
|
||||||
@@ -203,17 +205,75 @@ export class DocumentsController {
|
|||||||
return reply.code(400).send({ error: 'Bad Request', message: 'No file provided' });
|
return reply.code(400).send({ error: 'Bad Request', message: 'No file provided' });
|
||||||
}
|
}
|
||||||
|
|
||||||
const allowed = new Set(['application/pdf', 'image/jpeg', 'image/png']);
|
// 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;
|
const contentType = mp.mimetype as string | undefined;
|
||||||
if (!contentType || !allowed.has(contentType)) {
|
if (!contentType || !allowedTypes.has(contentType)) {
|
||||||
logger.warn('Unsupported file type for upload', {
|
logger.warn('Unsupported file type for upload (header validation)', {
|
||||||
operation: 'documents.upload.unsupported_type',
|
operation: 'documents.upload.unsupported_type',
|
||||||
user_id: userId,
|
user_id: userId,
|
||||||
document_id: documentId,
|
document_id: documentId,
|
||||||
content_type: contentType,
|
content_type: contentType,
|
||||||
file_name: mp.filename,
|
file_name: mp.filename,
|
||||||
});
|
});
|
||||||
return reply.code(415).send({ error: 'Unsupported Media Type' });
|
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 FileType.fromBuffer(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 originalName: string = mp.filename || 'upload';
|
||||||
@@ -235,10 +295,19 @@ export class DocumentsController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const counter = new CountingStream();
|
const counter = new CountingStream();
|
||||||
mp.file.pipe(counter);
|
|
||||||
|
// 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 storage = getStorageService();
|
||||||
const bucket = 'documents'; // Filesystem storage ignores bucket, but keep for interface compatibility
|
const bucket = 'documents';
|
||||||
const version = 'v1';
|
const version = 'v1';
|
||||||
const unique = cryptoRandom();
|
const unique = cryptoRandom();
|
||||||
const key = `documents/${userId}/${doc.vehicle_id}/${doc.id}/${version}/${unique}.${ext}`;
|
const key = `documents/${userId}/${doc.vehicle_id}/${doc.id}/${version}/${unique}.${ext}`;
|
||||||
@@ -261,6 +330,7 @@ export class DocumentsController {
|
|||||||
vehicle_id: doc.vehicle_id,
|
vehicle_id: doc.vehicle_id,
|
||||||
file_name: originalName,
|
file_name: originalName,
|
||||||
content_type: contentType,
|
content_type: contentType,
|
||||||
|
detected_type: detectedType.mime,
|
||||||
file_size: counter.bytes,
|
file_size: counter.bytes,
|
||||||
storage_key: key,
|
storage_key: key,
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user