Harden file signature validation and improve security logging#51
Conversation
Co-authored-by: Krosebrook <214532761+Krosebrook@users.noreply.github.com>
Co-authored-by: Krosebrook <214532761+Krosebrook@users.noreply.github.com>
Co-authored-by: Krosebrook <214532761+Krosebrook@users.noreply.github.com>
…to copilot/sub-pr-48
0b53a50
into
sentinel-fix-file-upload-bypass-2628575097628889752
There was a problem hiding this comment.
Pull request overview
This PR hardens file upload security by improving audio file signature validation and adding security logging for validation failures. It addresses feedback from PR #48 by closing validation gaps that could allow malicious files to bypass MIME-type checks.
Changes:
- Expands MP3 detection to cover MPEG-1 (FF FA), MPEG-2 (FF F2), and MPEG-2.5 (FF E2, FF E3) sync frames beyond the original MPEG-2 patterns
- Fixes M4A validation bypass by checking the major brand field (bytes 8-11) and rejecting generic MP4 container brands (isom, mp41, mp42) while accepting only audio-specific brands (M4A, M4B, M4P)
- Enhances
sanitizeLogutility to strip newlines from all string values, preventing log injection attacks across the codebase - Adds security logging for file signature validation failures with sanitized metadata (userId, fileSize, mimeType, originalName) for incident response
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/utils.ts | Adds newline stripping to sanitizeLog to prevent log injection attacks |
| server/utils.test.ts | Adds test coverage for log injection prevention and new audio signature patterns (MP3 variants, M4A brands, MP4 rejection) |
| server/routes.ts | Imports sanitizeLog and logs validation failures with sanitized request metadata for security monitoring |
| { | ||
| name: "Valid M4B (ftyp)", | ||
| input: Buffer.from([0x00, 0x00, 0x00, 0x1C, 0x66, 0x74, 0x79, 0x70, 0x4D, 0x34, 0x42, 0x20]), | ||
| expected: true, | ||
| }, |
There was a problem hiding this comment.
There are two nearly identical test cases for M4B format. The test at lines 119-122 is named "Valid M4A (M4B brand)" (existing test) and the newly added test at lines 124-126 is named "Valid M4B (ftyp)". Both test the M4B brand with the same signature pattern, differing only in the box size prefix (0x20 vs 0x1C). Consider consolidating these into a single test case or clarifying the distinction in the test names if the different box sizes represent distinct test scenarios.
Addresses code review feedback on PR #48's file upload validation security fix. The original implementation had validation gaps and lacked security monitoring.
Changes
File signature validation
Security monitoring
sanitizeLogutility to strip newlines from all strings, preventing log injection attacks across the codebaseCode quality
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by cubic
Fixes a file upload validation bypass with stricter magic‑byte checks and safe, sanitized logging. Expands audio signature validation and adds tests for newline stripping and new formats.
Written for commit a7143da. Summary will update on new commits.