🛡️ Sentinel: Fix file upload vulnerability by verifying magic bytes#58
🛡️ Sentinel: Fix file upload vulnerability by verifying magic bytes#58Krosebrook wants to merge 2 commits intomainfrom
Conversation
- Refactored detectAudioFormat to server/utils.ts - Added FLAC and AAC support to magic byte detection - Enforced magic byte validation on /api/audio/generate-with-reference endpoint Co-authored-by: Krosebrook <214532761+Krosebrook@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @Krosebrook, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a critical security enhancement by addressing a file upload vulnerability in the audio generation endpoint. It centralizes and significantly improves audio file format detection using magic bytes, moving away from unreliable client-provided MIME types. This robust server-side validation prevents the upload and processing of invalid or malicious files, thereby bolstering the application's security posture. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughCentralizes audio format detection by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully addresses the file upload vulnerability by implementing magic byte verification for audio uploads. Moving the detection logic to a centralized utility and expanding support for FLAC and AAC are excellent improvements for both security and maintainability. I have provided a suggestion to refine the WAV detection logic to ensure it specifically identifies WAV files among other RIFF-based formats, which further strengthens the security gate.
| if ( | ||
| (buffer[0] === 0xff && (buffer[1] === 0xfb || buffer[1] === 0xfa || buffer[1] === 0xf3)) || | ||
| (buffer[0] === 0x49 && buffer[1] === 0x44 && buffer[2] === 0x33) | ||
| ) { |
There was a problem hiding this comment.
The MP3 detection logic is quite restrictive, checking only for specific frame sync patterns and ID3 tags. Because the AAC check (line 84) is broader and follows this check, many valid MP3 files that don't match these specific patterns will be misidentified as aac. While this still passes the security gate, it will cause ensureCompatibleFormat to unnecessarily convert these MP3 files to WAV, impacting performance.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 417ae85f07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| // AAC: ADTS header (FFF1 or FFF9 usually) | ||
| // Sync word 111111111111 (12 bits) | ||
| if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) { |
There was a problem hiding this comment.
The new upload guard in server/routes.ts accepts any file when detectAudioFormat(...) !== "unknown", but this AAC branch treats any buffer starting with 0xFF plus a high nibble of 0xF as valid AAC. That pattern is broad enough that a crafted non-audio payload can be prefixed to satisfy the check and bypass the intended hardening, so the vulnerability fix is still bypassable for this endpoint when attackers control upload bytes and MIME type.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/routes.ts (1)
1151-1155: Good security fix — but consider using the detected format for the data URL too.Line 1163 still uses the client-controlled
file.mimetypeto build thedataUrl. After validating magic bytes, you could derive the correct MIME type fromdetectedFormatinstead, closing the remaining gap where a user could supply a misleading MIME type (e.g.,audio/wavfor an actual MP3).♻️ Suggested improvement
+ const mimeMap: Record<string, string> = { + wav: "audio/wav", + mp3: "audio/mpeg", + webm: "audio/webm", + mp4: "audio/mp4", + ogg: "audio/ogg", + flac: "audio/flac", + aac: "audio/aac", + }; + const base64 = file.buffer.toString("base64"); - const dataUrl = `data:${file.mimetype};base64,${base64}`; + const dataUrl = `data:${mimeMap[detectedFormat] ?? file.mimetype};base64,${base64}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes.ts` around lines 1151 - 1155, After verifying audio magic bytes with detectAudioFormat and assigning detectedFormat, stop using the client-supplied file.mimetype when constructing the data URL; instead map detectedFormat to the correct MIME type and use that value for the dataUrl. Locate where dataUrl is built (references to dataUrl and file.mimetype) and replace the mimetype source with a safe mapping derived from detectedFormat (e.g., "audio/mpeg" for "mp3", "audio/wav" for "wav"), preserving the base64 encoding logic.server/utils.ts (1)
63-86: MP3 frame-sync and AAC ADTS detection overlap.The MP3 check at lines 64-68 only covers three specific sync-word second bytes (
0xFB,0xFA,0xF3). Other valid MP3 frame-sync patterns (e.g.,0xFF 0xF2for MPEG 2.5 Layer III) will fall through and be misidentified as AAC by the broader check at line 84. This doesn't affect the security goal (both are valid audio), but could cause incorrect format labeling downstream.A more precise ADTS check would also verify that the layer bits (bits 1–2 of byte 1) are
00:♻️ Optional: tighten the AAC check
- // AAC: ADTS header (FFF1 or FFF9 usually) - // Sync word 111111111111 (12 bits) - if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) { + // AAC: ADTS sync word (12 bits = 0xFFF) with layer bits == 00 + if (buffer[0] === 0xff && (buffer[1] & 0xf6) === 0xf0) { return "aac"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils.ts` around lines 63 - 86, The MP3 detection is too narrow (only checks buffer[1] === 0xFB/0xFA/0xF3) and the subsequent AAC ADTS check ((buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0)) will mislabel some MP3 variants as AAC; update the checks in the detection block so MP3 frame-sync covers all valid second-byte sync patterns (e.g., any 0xE0-0xFF values with appropriate MPEG/version/layer bits) or tighten the AAC ADTS check by verifying the ADTS layer bits are 00 (bits 1–2 of buffer[1]) before returning "aac"; locate and modify the existing MP3/ADTS conditionals in server/utils.ts (the buffer[...] checks) to implement one of these fixes so MP3 variants like 0xFF 0xF2 are correctly identified as "mp3".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/routes.ts`:
- Around line 1151-1155: After verifying audio magic bytes with
detectAudioFormat and assigning detectedFormat, stop using the client-supplied
file.mimetype when constructing the data URL; instead map detectedFormat to the
correct MIME type and use that value for the dataUrl. Locate where dataUrl is
built (references to dataUrl and file.mimetype) and replace the mimetype source
with a safe mapping derived from detectedFormat (e.g., "audio/mpeg" for "mp3",
"audio/wav" for "wav"), preserving the base64 encoding logic.
In `@server/utils.ts`:
- Around line 63-86: The MP3 detection is too narrow (only checks buffer[1] ===
0xFB/0xFA/0xF3) and the subsequent AAC ADTS check ((buffer[0] === 0xff &&
(buffer[1] & 0xf0) === 0xf0)) will mislabel some MP3 variants as AAC; update the
checks in the detection block so MP3 frame-sync covers all valid second-byte
sync patterns (e.g., any 0xE0-0xFF values with appropriate MPEG/version/layer
bits) or tighten the AAC ADTS check by verifying the ADTS layer bits are 00
(bits 1–2 of buffer[1]) before returning "aac"; locate and modify the existing
MP3/ADTS conditionals in server/utils.ts (the buffer[...] checks) to implement
one of these fixes so MP3 variants like 0xFF 0xF2 are correctly identified as
"mp3".
There was a problem hiding this comment.
Pull request overview
This PR addresses a critical file upload security vulnerability in the /api/audio/generate-with-reference endpoint by implementing magic byte validation. Previously, the endpoint relied solely on client-controlled MIME types, which could allow malicious file uploads. The fix refactors the detectAudioFormat function from a specialized module to a shared utility, extends it to support FLAC and AAC formats, and applies it to verify uploaded file signatures before processing.
Changes:
- Centralized audio format detection by moving
detectAudioFormatfromserver/replit_integrations/audio/client.tstoserver/utils.ts - Enhanced format detection to support FLAC and AAC magic bytes
- Added magic byte verification to the file upload endpoint to prevent malicious uploads
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| server/utils.ts | Added detectAudioFormat function with magic byte detection for WAV, MP3, WebM, MP4, OGG, FLAC, and AAC formats |
| server/routes.ts | Implemented file signature validation using detectAudioFormat before processing uploaded audio files |
| server/replit_integrations/audio/client.ts | Refactored to import and re-export detectAudioFormat from utils.ts for backward compatibility |
| // Sync word 111111111111 (12 bits) | ||
| if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) { |
There was a problem hiding this comment.
The AAC detection pattern (checking if buffer[0] is 0xff and buffer[1] & 0xf0 equals 0xf0) can false-positive on MP3 files that start with frame sync patterns like 0xFFFA, 0xFFFB, or 0xFFF3. Since MP3 is checked earlier, this won't cause misidentification, but it means some MP3 files could theoretically match both patterns. Consider adding a comment explaining that the order of checks matters, or make the AAC check more specific by excluding known MP3 sync patterns.
| // Sync word 111111111111 (12 bits) | |
| if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) { | |
| // Sync word 111111111111 (12 bits). Exclude known MP3 sync patterns already handled above. | |
| if ( | |
| buffer[0] === 0xff && | |
| (buffer[1] & 0xf0) === 0xf0 && | |
| buffer[1] !== 0xfa && | |
| buffer[1] !== 0xfb && | |
| buffer[1] !== 0xf3 | |
| ) { |
| export function detectAudioFormat(buffer: Buffer): AudioFormat { | ||
| if (buffer.length < 12) return "unknown"; | ||
|
|
||
| // WAV: RIFF....WAVE | ||
| if (buffer[0] === 0x52 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x46) { | ||
| return "wav"; | ||
| } | ||
| // WebM: EBML header | ||
| if (buffer[0] === 0x1a && buffer[1] === 0x45 && buffer[2] === 0xdf && buffer[3] === 0xa3) { | ||
| return "webm"; | ||
| } | ||
| // MP3: ID3 tag or frame sync | ||
| if ( | ||
| (buffer[0] === 0xff && (buffer[1] === 0xfb || buffer[1] === 0xfa || buffer[1] === 0xf3)) || | ||
| (buffer[0] === 0x49 && buffer[1] === 0x44 && buffer[2] === 0x33) | ||
| ) { | ||
| return "mp3"; | ||
| } | ||
| // MP4/M4A/MOV: ....ftyp | ||
| if (buffer[4] === 0x66 && buffer[5] === 0x74 && buffer[6] === 0x79 && buffer[7] === 0x70) { | ||
| return "mp4"; | ||
| } | ||
| // OGG: OggS | ||
| if (buffer[0] === 0x4f && buffer[1] === 0x67 && buffer[2] === 0x67 && buffer[3] === 0x53) { | ||
| return "ogg"; | ||
| } | ||
| // FLAC: fLaC | ||
| if (buffer[0] === 0x66 && buffer[1] === 0x4c && buffer[2] === 0x61 && buffer[3] === 0x43) { | ||
| return "flac"; | ||
| } | ||
| // AAC: ADTS header (FFF1 or FFF9 usually) | ||
| // Sync word 111111111111 (12 bits) | ||
| if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) { | ||
| return "aac"; | ||
| } | ||
|
|
||
| return "unknown"; | ||
| } |
There was a problem hiding this comment.
The new detectAudioFormat function lacks test coverage. The existing utils.test.ts file contains tests for sanitizeLog, demonstrating that utility functions in this file are tested. While the PR description mentions a test_audio_detect.ts file with 7/7 tests passing, this test file is not included in the repository. Consider adding tests for detectAudioFormat to utils.test.ts to ensure all supported formats (WAV, MP3, WebM, MP4, OGG, FLAC, AAC) are correctly detected and that unknown formats return "unknown".
|
|
||
| // Sentinel: Verify file content matches audio signature (prevent malicious uploads) | ||
| const detectedFormat = detectAudioFormat(file.buffer); | ||
| if (detectedFormat === "unknown") { |
There was a problem hiding this comment.
Security-critical validation failures like file signature mismatches should be logged with sanitized metadata for security monitoring and incident response. According to established codebase conventions, such failures should be logged with information like userId, fileSize, mimeType, and originalName using the sanitizeLog utility. Consider adding logging here similar to: console.warn("File signature validation failed:", sanitizeLog({ userId: req.user.claims.sub, fileSize: file.size, mimeType: file.mimetype, originalName: file.originalname })).
| if (detectedFormat === "unknown") { | |
| if (detectedFormat === "unknown") { | |
| console.warn( | |
| "File signature validation failed:", | |
| sanitizeLog({ | |
| userId: req.user?.claims?.sub, | |
| fileSize: file.size, | |
| mimeType: file.mimetype, | |
| originalName: file.originalname, | |
| }) | |
| ); |
| // WAV: RIFF....WAVE | ||
| if (buffer[0] === 0x52 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x46) { | ||
| return "wav"; | ||
| } |
There was a problem hiding this comment.
The WAV format detection only checks for the RIFF header (bytes 0-3) but does not verify the WAVE signature at bytes 8-11. A complete WAV file should have "RIFF" at bytes 0-3 and "WAVE" at bytes 8-11. Other RIFF-based formats (like AVI video files) also start with RIFF but have different format identifiers. Consider adding a check for the WAVE signature to prevent false positives: if (buffer[0] === 0x52 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x46 && buffer[8] === 0x57 && buffer[9] === 0x41 && buffer[10] === 0x56 && buffer[11] === 0x45)
| return res.status(400).json({ message: "Reference audio file is required" }); | ||
| } | ||
|
|
||
| // Sentinel: Verify file content matches audio signature (prevent malicious uploads) |
There was a problem hiding this comment.
The comment prefix "Sentinel:" violates the codebase convention. According to established guidelines, branded prefixes like "Tool:", "AI:", or "Sentinel:" should not be used. Use a simple descriptive comment instead.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/utils.ts">
<violation number="1" location="server/utils.ts:56">
P2: WAV detection only checks the RIFF container header, so non-audio RIFF files (e.g., AVI/WEBP) would be misclassified as WAV. Verify the "WAVE" identifier at bytes 8–11 to avoid accepting non-audio files.</violation>
<violation number="2" location="server/utils.ts:84">
P1: The AAC signature check is too permissive - it accepts any buffer starting with `0xFF` followed by a byte with high nibble `0xF`. This pattern is broad enough that a crafted malicious payload can be prefixed to satisfy the check, bypassing the intended security hardening. Consider implementing stricter ADTS frame validation (e.g., verify the full sync word and check frame header fields like profile, sample rate).</violation>
</file>
<file name="server/routes.ts">
<violation number="1" location="server/routes.ts:1152">
P2: The new magic-byte check only rejects `unknown`, but it doesn't validate that the detected format matches the client-provided `file.mimetype`. A spoofed MIME can still pass verification and produce a mismatched data URL. Validate that the MIME is consistent with `detectedFormat` (or derive the MIME from `detectedFormat`) before proceeding.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| // AAC: ADTS header (FFF1 or FFF9 usually) | ||
| // Sync word 111111111111 (12 bits) | ||
| if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) { |
There was a problem hiding this comment.
P1: The AAC signature check is too permissive - it accepts any buffer starting with 0xFF followed by a byte with high nibble 0xF. This pattern is broad enough that a crafted malicious payload can be prefixed to satisfy the check, bypassing the intended security hardening. Consider implementing stricter ADTS frame validation (e.g., verify the full sync word and check frame header fields like profile, sample rate).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/utils.ts, line 84:
<comment>The AAC signature check is too permissive - it accepts any buffer starting with `0xFF` followed by a byte with high nibble `0xF`. This pattern is broad enough that a crafted malicious payload can be prefixed to satisfy the check, bypassing the intended security hardening. Consider implementing stricter ADTS frame validation (e.g., verify the full sync word and check frame header fields like profile, sample rate).</comment>
<file context>
@@ -40,3 +42,48 @@ export function sanitizeLog(data: any): any {
+ }
+ // AAC: ADTS header (FFF1 or FFF9 usually)
+ // Sync word 111111111111 (12 bits)
+ if (buffer[0] === 0xff && (buffer[1] & 0xf0) === 0xf0) {
+ return "aac";
+ }
</file context>
| const detectedFormat = detectAudioFormat(file.buffer); | ||
| if (detectedFormat === "unknown") { | ||
| return res.status(400).json({ message: "Invalid audio file format (verification failed)" }); | ||
| } |
There was a problem hiding this comment.
P2: The new magic-byte check only rejects unknown, but it doesn't validate that the detected format matches the client-provided file.mimetype. A spoofed MIME can still pass verification and produce a mismatched data URL. Validate that the MIME is consistent with detectedFormat (or derive the MIME from detectedFormat) before proceeding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes.ts, line 1152:
<comment>The new magic-byte check only rejects `unknown`, but it doesn't validate that the detected format matches the client-provided `file.mimetype`. A spoofed MIME can still pass verification and produce a mismatched data URL. Validate that the MIME is consistent with `detectedFormat` (or derive the MIME from `detectedFormat`) before proceeding.</comment>
<file context>
@@ -1147,6 +1148,12 @@ Also suggest a fitting title for the song.`;
}
+ // Sentinel: Verify file content matches audio signature (prevent malicious uploads)
+ const detectedFormat = detectAudioFormat(file.buffer);
+ if (detectedFormat === "unknown") {
+ return res.status(400).json({ message: "Invalid audio file format (verification failed)" });
</file context>
| const detectedFormat = detectAudioFormat(file.buffer); | |
| if (detectedFormat === "unknown") { | |
| return res.status(400).json({ message: "Invalid audio file format (verification failed)" }); | |
| } | |
| const detectedFormat = detectAudioFormat(file.buffer); | |
| const allowedMimesByFormat: Record<string, string[]> = { | |
| wav: ["audio/wav", "audio/x-wav"], | |
| mp3: ["audio/mpeg", "audio/mp3"], | |
| ogg: ["audio/ogg"], | |
| flac: ["audio/flac"], | |
| mp4: ["audio/mp4"], | |
| aac: ["audio/aac"], | |
| }; | |
| if (detectedFormat === "unknown" || !allowedMimesByFormat[detectedFormat]?.includes(file.mimetype)) { | |
| return res.status(400).json({ message: "Invalid audio file format (verification failed)" }); | |
| } |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/utils.ts (1)
55-56: Remove the duplicate comment.
// WAV: RIFF....WAVEappears on both Line 55 and Line 56.🔧 Proposed fix
- // WAV: RIFF....WAVE // WAV: RIFF....WAVE if (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils.ts` around lines 55 - 56, There is a duplicated inline comment "// WAV: RIFF....WAVE" in server/utils.ts; remove the redundant occurrence so only a single "// WAV: RIFF....WAVE" comment remains (leave one copy to preserve the intent), ensuring no other nearby comments or code are altered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/utils.ts`:
- Around line 68-73: The MP3 frame-sync check in server/utils.ts currently
matches only a few literal second-byte values and misses MPEG2.5/other Layer3
variants; replace the literal checks in the condition that returns "mp3" with a
bitmask-based test: when buffer[0] === 0xff, test (buffer[1] & 0xE6) === 0xE2 to
match all MPEG Layer3 combinations (covers 0xE2/0xE3/0xF2/0xF3/0xFA/0xFB),
leaving version and CRC bits unconstrained; keep the existing ID3 check and
ensure this bitmask check remains before the AAC classification.
---
Nitpick comments:
In `@server/utils.ts`:
- Around line 55-56: There is a duplicated inline comment "// WAV: RIFF....WAVE"
in server/utils.ts; remove the redundant occurrence so only a single "// WAV:
RIFF....WAVE" comment remains (leave one copy to preserve the intent), ensuring
no other nearby comments or code are altered.
| if ( | ||
| (buffer[0] === 0xff && (buffer[1] === 0xfb || buffer[1] === 0xfa || buffer[1] === 0xf3)) || | ||
| (buffer[0] === 0x49 && buffer[1] === 0x44 && buffer[2] === 0x33) | ||
| ) { | ||
| return "mp3"; | ||
| } |
There was a problem hiding this comment.
MPEG2.5 Layer3 MP3 files are incorrectly rejected as "unknown", blocking valid uploads.
The frame-sync check only covers three specific second-byte values:
0xFB→ MPEG1 Layer3, no CRC0xFA→ MPEG1 Layer3, with CRC0xF3→ MPEG2 Layer3, no CRC
Two additional families are missing:
0xFF 0xF2(MPEG2 Layer3 with CRC) — skips the MP3 branch and is silently mislabeled as AAC (still accepted, just wrong label).0xFF 0xE3and0xFF 0xE2(MPEG2.5 Layer3) — skips both the MP3 branch (0xE? & 0xF0 = 0xE0 ≠ 0xF0) and the AAC branch, and falls through toreturn "unknown", causing legitimate MPEG2.5-encoded MP3 uploads to be rejected with a400.
MPEG 2.5 is an unofficial extension of MPEG 2 to support even lower sampling rates, so it appears in the wild on low-bitrate recordings. A frame sync byte sequence means finding a byte with value 255 followed by a byte with its three (or four) most significant bits set, which is why the version bits must be considered separately from the sync bits.
Replace the frame-sync literal list with a bitmask that checks only the sync field (top 3 bits of buffer[1]) and the layer field (bits 2–1 of buffer[1]), leaving the version bits and CRC bit free:
🛡️ Proposed fix — cover all MPEG Layer3 variants via bitmask
// MP3: ID3 tag or frame sync
if (
- (buffer[0] === 0xff && (buffer[1] === 0xfb || buffer[1] === 0xfa || buffer[1] === 0xf3)) ||
- (buffer[0] === 0x49 && buffer[1] === 0x44 && buffer[2] === 0x33)
+ // ID3v2 tag header
+ (buffer[0] === 0x49 && buffer[1] === 0x44 && buffer[2] === 0x33) ||
+ // MPEG frame sync (11 bits) + Layer III (bits 2-1 = 01), all versions (MPEG1/2/2.5) and CRC variants
+ (buffer[0] === 0xff && (buffer[1] & 0xe6) === 0xe2)
) {
return "mp3";
}The mask 0xE6 (0b1110_0110) checks sync[7:5]=111 and layer[2:1]=01; 0xE2 is the expected result. This matches 0xE2, 0xE3, 0xF2, 0xF3, 0xFA, and 0xFB, covering every defined MPEG Layer3 combination. Placing this before the AAC check (as it already is) ensures there is no false AAC classification either.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils.ts` around lines 68 - 73, The MP3 frame-sync check in
server/utils.ts currently matches only a few literal second-byte values and
misses MPEG2.5/other Layer3 variants; replace the literal checks in the
condition that returns "mp3" with a bitmask-based test: when buffer[0] === 0xff,
test (buffer[1] & 0xE6) === 0xE2 to match all MPEG Layer3 combinations (covers
0xE2/0xE3/0xF2/0xF3/0xFA/0xFB), leaving version and CRC bits unconstrained; keep
the existing ID3 check and ensure this bitmask check remains before the AAC
classification.
1 participant
Vulnerability: The
/api/audio/generate-with-referenceendpoint relied onfile.mimetype(client-controlled) to validate uploads, allowing potential malicious file uploads (e.g. executables).Fix:
detectAudioFormatfromserver/replit_integrations/audio/client.tstoserver/utils.ts.detectAudioFormatto support FLAC and AAC magic bytes.server/routes.tsto usedetectAudioFormatto verify the actual file buffer before processing.server/replit_integrations/audio/client.ts.Verification:
server/test_audio_detect.ts(7/7 passed for WAV, MP3, FLAC, AAC, EXE, Empty, Short).pnpm checkto ensure type safety.PR created automatically by Jules for task 11126110579953094161 started by @Krosebrook
Summary by cubic
Secures /api/audio/generate-with-reference by verifying uploaded files with magic bytes instead of mimetype. Blocks non-audio files and adds FLAC/AAC support.
Bug Fixes
Refactors
Written for commit a30ec5a. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
New Features