diff --git a/.jules/sentinel.md b/.jules/sentinel.md index c7a67127..1564102a 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,8 @@ **Vulnerability:** CSV formula injection mitigation was naive, missing leading whitespace, tabs, and newlines. **Learning:** Checking `/^[=+\-@]/` is not sufficient, as OWASP states that spaces and tabs before the formula triggers will also execute the formula in applications like Excel. **Prevention:** Use a regex that allows leading whitespace (e.g. `/^[\s\uFEFF\xA0]*[=+\-@\t\r\n]/`) and include standalone tabs or new lines which are also injection vectors. + +## 2025-06-22 - URL Parsing Length Limit +**Vulnerability:** Unbounded URL inputs across TypeScript, Rust, and Python entry points. +**Learning:** Regular expressions and URL parsers can spend avoidable CPU or memory on oversized attacker-controlled strings. +**Prevention:** Cap URL length to the product-supported maximum before handing user input to regex or URL parsers. diff --git a/apps/desktop/src-tauri/src/main.rs b/apps/desktop/src-tauri/src/main.rs index 2de7adde..ee47cc26 100644 --- a/apps/desktop/src-tauri/src/main.rs +++ b/apps/desktop/src-tauri/src/main.rs @@ -34,6 +34,7 @@ const ANALYSIS_WAIT_POLL: Duration = Duration::from_millis(50); const AUDIO_EXTENSIONS: [&str; 4] = ["wav", "mp3", "flac", "m4a"]; const MISSING_ANALYSIS_PYTHON: &str = "__bandscope_missing_analysis_python__"; const YOUTUBE_IMPORT_TIMEOUT: Duration = Duration::from_secs(120); +const MAX_YOUTUBE_URL_LENGTH: usize = 2000; impl Default for AppState { fn default() -> Self { @@ -1057,6 +1058,10 @@ async fn import_youtube_url( } fn is_supported_youtube_url(url: &str) -> bool { + if url.len() > MAX_YOUTUBE_URL_LENGTH { + return false; + } + let parsed_url = match url::Url::parse(url) { Ok(u) => u, Err(_) => return false, @@ -1366,6 +1371,11 @@ mod tests { assert!(!is_supported_youtube_url( "https://youtube.com/watch?v=abc123DEF45&v=def456GHI78" )); + let oversized_url = format!( + "https://youtube.com/watch?v=abc123DEF45&x={}", + "a".repeat(MAX_YOUTUBE_URL_LENGTH) + ); + assert!(!is_supported_youtube_url(&oversized_url)); assert!(!is_supported_youtube_url("https://youtu.be/abc123")); assert!(!is_supported_youtube_url("https://youtu.be/abc123DEF4!")); } diff --git a/apps/desktop/src/App.test.tsx b/apps/desktop/src/App.test.tsx index c039dfba..7e2b6d8a 100644 --- a/apps/desktop/src/App.test.tsx +++ b/apps/desktop/src/App.test.tsx @@ -223,6 +223,12 @@ describe("App", () => { expect(sourceControls).toHaveTextContent(/Import YouTube/i); }); + it("caps the YouTube URL input before import-path validation", () => { + render(); + + expect(screen.getByRole("textbox", { name: /YouTube URL/i })).toHaveAttribute("maxlength", "2000"); + }); + it("renders the loaded song as a dark rehearsal command board", async () => { mockLoadProject.mockResolvedValueOnce(succeededResult().result); render(); diff --git a/apps/desktop/src/App.tsx b/apps/desktop/src/App.tsx index 24f5fb09..b633b662 100644 --- a/apps/desktop/src/App.tsx +++ b/apps/desktop/src/App.tsx @@ -35,6 +35,7 @@ import { importYoutubeUrl, isSupportedYoutubeUrl, loadProject, + MAX_YOUTUBE_URL_LENGTH, saveProject, subscribeToAnalysisJobUpdates, selectLocalAudioSource, @@ -605,6 +606,7 @@ export function App() { type="text" placeholder={t("youtubePlaceholder")} value={youtubeUrl} + maxLength={MAX_YOUTUBE_URL_LENGTH} onChange={(e) => setYoutubeUrl(e.target.value)} disabled={analysisInFlight || isStarting || isImporting} className="h-10 flex-1 border-0 bg-transparent text-slate-100 placeholder:text-slate-500 focus-visible:ring-cyan-300" diff --git a/apps/desktop/src/lib/analysis.test.ts b/apps/desktop/src/lib/analysis.test.ts index b443b41f..e3347d1f 100644 --- a/apps/desktop/src/lib/analysis.test.ts +++ b/apps/desktop/src/lib/analysis.test.ts @@ -1,6 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createDemoAnalysisJobRequest, createDemoRehearsalSong } from "@bandscope/shared-types"; -import { getAnalysisJobStatus, importYoutubeUrl, startAnalysisJob } from "./analysis"; +import { + MAX_YOUTUBE_URL_LENGTH, + getAnalysisJobStatus, + importYoutubeUrl, + startAnalysisJob +} from "./analysis"; type TauriWindow = Window & { __TAURI_INTERNALS__?: unknown; @@ -196,6 +201,23 @@ describe("analysis bridge", () => { }); }); + it("rejects oversized YouTube URLs before crossing the Tauri bridge", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn(); + const urlPrefix = "https://youtube.com/watch?v=4ozX4yFUC34&x="; + const oversizedUrl = `${urlPrefix}${"a".repeat(MAX_YOUTUBE_URL_LENGTH - urlPrefix.length + 1)}`; + + const selection = await importYoutubeUrl(oversizedUrl); + + expect(tauriWindow.__TAURI_INVOKE__).not.toHaveBeenCalled(); + expect(selection).toEqual({ + ok: false, + error: { + code: "invalid_request", + message: "Only standard YouTube URLs are supported." + } + }); + }); + it.each([ "https://youtube.com/watch?v=too-short", "https://youtube.com/watch?v=4ozX4yFUC3!", diff --git a/apps/desktop/src/lib/analysis.ts b/apps/desktop/src/lib/analysis.ts index f6ee7c00..bb750b34 100644 --- a/apps/desktop/src/lib/analysis.ts +++ b/apps/desktop/src/lib/analysis.ts @@ -43,6 +43,9 @@ const SAFE_LOCAL_AUDIO_MESSAGES = new Set([ "Could not prepare the local temp workspace." ]); const YOUTUBE_VIDEO_ID_PATTERN = /^[A-Za-z0-9_-]{11}$/; +const MAX_YOUTUBE_URL_LENGTH = 2000; + +export { MAX_YOUTUBE_URL_LENGTH }; /** Documented. */ export type LocalAudioSelectionResult = @@ -74,6 +77,9 @@ export function isSupportedYoutubeUrl(rawUrl: unknown): rawUrl is string { if (typeof rawUrl !== "string") { return false; } + if (rawUrl.length > MAX_YOUTUBE_URL_LENGTH) { + return false; + } let parsedUrl: URL; try { diff --git a/services/analysis-engine/src/bandscope_analysis/youtube.py b/services/analysis-engine/src/bandscope_analysis/youtube.py index aa27d6f7..c98f4e51 100644 --- a/services/analysis-engine/src/bandscope_analysis/youtube.py +++ b/services/analysis-engine/src/bandscope_analysis/youtube.py @@ -15,6 +15,7 @@ import yt_dlp # type: ignore YOUTUBE_VIDEO_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]{11}$") +MAX_YOUTUBE_URL_LENGTH = 2000 SUPPORTED_AUDIO_EXTENSIONS = (".opus", ".m4a", ".mp3", ".wav", ".aac", ".flac", ".ogg") YOUTUBE_DOWNLOAD_FAILED_MESSAGE = ( "Failed to download audio from YouTube. Please use a local audio file instead." @@ -32,6 +33,10 @@ def validate_url(url: str) -> bool: Returns: True if the URL is valid, False otherwise. """ + # Pragmatic upper bound to avoid spending parser/downloader work on oversized user input. + if len(url) > MAX_YOUTUBE_URL_LENGTH: + return False + try: parsed = urllib.parse.urlparse(url) if parsed.scheme != "https": diff --git a/services/analysis-engine/tests/test_youtube.py b/services/analysis-engine/tests/test_youtube.py index 71d7d60d..5531ac9d 100644 --- a/services/analysis-engine/tests/test_youtube.py +++ b/services/analysis-engine/tests/test_youtube.py @@ -7,7 +7,7 @@ import pytest import yt_dlp # type: ignore -from bandscope_analysis.youtube import download_youtube_audio, validate_url +from bandscope_analysis.youtube import MAX_YOUTUBE_URL_LENGTH, download_youtube_audio, validate_url def test_validate_url() -> None: @@ -16,7 +16,11 @@ def test_validate_url() -> None: assert validate_url("https://youtu.be/abc123DEF45") is True assert validate_url("https://www.youtube.com/watch?v=abc123DEF45") is True assert validate_url("https://www.youtube.com/watch?v=abc123DEF45&t=10") is True + url_prefix = "https://youtube.com/watch?v=abc123DEF45&x=" + max_length_url = url_prefix + ("a" * (MAX_YOUTUBE_URL_LENGTH - len(url_prefix))) + long_query_url = max_length_url + "a" + assert validate_url(max_length_url) is True assert validate_url("https://m.youtube.com/watch?v=abc123DEF45") is False assert validate_url("https://music.youtube.com/watch?v=abc123DEF45") is False assert validate_url("https://evil.youtube.com/watch?v=abc123DEF45") is False @@ -34,6 +38,7 @@ def test_validate_url() -> None: assert validate_url("https://youtube.com/watch?v=abc123DEF45&v=") is False assert validate_url("https://youtube.com/watch?v=../../../etc/passwd") is False assert validate_url("https://youtu.be/../../../etc/passwd") is False + assert validate_url(long_query_url) is False def test_validate_url_edge_cases() -> None: