From 469faed7b91265302e49ef1f3049e11303748a15 Mon Sep 17 00:00:00 2001 From: NimBold Date: Thu, 16 Jul 2026 22:20:12 +0330 Subject: [PATCH] fix(downloads): harden add flow and URL validation --- src-tauri/src/lib.rs | 131 +++++++++++++++------ src/components/AddDownloadsModal.tsx | 37 ++++-- src/components/KeychainPermissionModal.tsx | 54 ++++++--- src/components/Sidebar.tsx | 30 ++++- src/store/useDownloadStore.test.ts | 85 ++++++++++++- src/store/useDownloadStore.ts | 98 ++++++++++++--- src/utils/addDownloadMetadata.test.ts | 6 + src/utils/addDownloadMetadata.ts | 14 ++- 8 files changed, 375 insertions(+), 80 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 804fde7..3bd091c 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1527,42 +1527,52 @@ fn should_cleanup_media_artifacts_after_failure( !(crate::retry::is_transient_network_error(failure_reason) && strike < max_retries) } +fn is_blocked_network_address(ip: std::net::IpAddr) -> bool { + if ip.is_loopback() || ip.is_multicast() || ip.is_unspecified() { + return true; + } + match ip { + std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_link_local(), + std::net::IpAddr::V6(ipv6) => { + ipv6.to_ipv4().is_some_and(|ipv4| is_blocked_network_address(ipv4.into())) + || (ipv6.segments()[0] & 0xfe00) == 0xfc00 + || (ipv6.segments()[0] & 0xffc0) == 0xfe80 + } + } +} + +async fn resolve_and_validate_url_host( + parsed: &reqwest::Url, +) -> Result<(String, std::net::SocketAddr), String> { + let host = parsed.host_str().ok_or("SSRF blocked: No host")?; + let lookup_host = host.trim_start_matches('[').trim_end_matches(']'); + let port = parsed.port_or_known_default().unwrap_or_else(|| match parsed.scheme() { + "ftp" => 21, + "sftp" => 22, + _ => 80, + }); + + let addrs: Vec<_> = if let Ok(ip) = lookup_host.parse::() { + vec![std::net::SocketAddr::new(ip, port)] + } else { + tokio::net::lookup_host((lookup_host, port)) + .await + .map_err(|_| "SSRF blocked: DNS resolution failed")? + .collect() + }; + let addr = addrs.first().copied().ok_or("SSRF blocked: No DNS records")?; + if addrs.iter().any(|candidate| is_blocked_network_address(candidate.ip())) { + return Err("SSRF blocked: Private/local IP not allowed".to_string()); + } + Ok((lookup_host.to_string(), addr)) +} + async fn validate_url_ssrf(url: &str) -> Result, String> { let parsed = reqwest::Url::parse(url).map_err(|_| "SSRF blocked: Invalid URL")?; if parsed.scheme() != "http" && parsed.scheme() != "https" { return Err("SSRF blocked: Only HTTP/HTTPS schemes allowed".to_string()); } - let host = parsed.host_str().ok_or("SSRF blocked: No host")?; - let port = parsed.port_or_known_default().unwrap_or(80); - - let mut addrs = tokio::net::lookup_host((host, port)) - .await - .map_err(|_| "SSRF blocked: DNS resolution failed")?; - - let addr = addrs.next().ok_or("SSRF blocked: No DNS records")?; - let ip = addr.ip(); - - if ip.is_loopback() || ip.is_multicast() || ip.is_unspecified() { - return Err("SSRF blocked: Private/local IP not allowed".to_string()); - } - match ip { - std::net::IpAddr::V4(ipv4) => { - if ipv4.is_private() || ipv4.is_link_local() { - return Err("SSRF blocked: Private/local IP not allowed".to_string()); - } - } - std::net::IpAddr::V6(ipv6) => { - if (ipv6.segments()[0] & 0xfe00) == 0xfc00 { - // ULA check - return Err("SSRF blocked: Private/local IP not allowed".to_string()); - } - if (ipv6.segments()[0] & 0xffc0) == 0xfe80 { - // Link-local check - return Err("SSRF blocked: Private/local IP not allowed".to_string()); - } - } - } - Ok(Some((host.to_string(), addr))) + resolve_and_validate_url_host(&parsed).await.map(Some) } fn same_origin(left: &reqwest::Url, right: &reqwest::Url) -> bool { @@ -1632,7 +1642,15 @@ async fn fetch_metadata( builder = builder.user_agent("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/150.0.0.0 Safari/537.36"); } - let resolved_addr = validate_url_ssrf(¤t_url).await?; + let parsed_current_url = reqwest::Url::parse(¤t_url) + .map_err(|_| "SSRF blocked: Invalid URL".to_string())?; + let resolved_addr = match parsed_current_url.scheme() { + "http" | "https" => validate_url_ssrf(¤t_url).await?, + "ftp" | "sftp" => resolve_and_validate_url_host(&parsed_current_url) + .await + .map(Some)?, + _ => return Err("Unsupported URL scheme".to_string()), + }; if let Some((host, addr)) = resolved_addr { builder = builder.resolve(&host, addr); @@ -4985,12 +5003,25 @@ fn enqueue_lifecycle_generation(item: &queue::EnqueueItem) -> Result Result<(), String> { + let parsed = reqwest::Url::parse(url).map_err(|_| "SSRF blocked: Invalid URL".to_string())?; + match parsed.scheme() { + "http" | "https" | "ftp" | "sftp" => { + resolve_and_validate_url_host(&parsed).await.map(|_| ()) + } + _ => Err("Unsupported URL scheme".to_string()), + } +} + #[tauri::command] async fn enqueue_download( app_handle: tauri::AppHandle, state: tauri::State<'_, AppState>, mut item: queue::EnqueueItem, ) -> Result { + validate_enqueue_url(&item.url) + .await + .map_err(AppError::Internal)?; let id = item.id.clone(); item.filename = crate::download_ownership::canonical_download_filename(&item.filename); let accepted_filename = item.filename.clone(); @@ -5054,8 +5085,17 @@ async fn enqueue_many( ) -> Result, AppError> { let mut results = Vec::with_capacity(items.len()); for mut item in items { - item.filename = crate::download_ownership::canonical_download_filename(&item.filename); let id = item.id.clone(); + if let Err(error) = validate_enqueue_url(&item.url).await { + results.push(crate::ipc::EnqueueResult { + id, + success: false, + filename: None, + error: Some(error), + }); + continue; + } + item.filename = crate::download_ownership::canonical_download_filename(&item.filename); let filename = item.filename.clone(); let lifecycle_generation = match enqueue_lifecycle_generation(&item) { Ok(generation) => generation, @@ -6062,10 +6102,35 @@ mod tests { observe_aria2_connections, observe_aria2_connections_with_epoch, Aria2ConnectionObservation, Aria2RecoveryReason, parse_media_playlist_metadata, + validate_enqueue_url, }; use serde_json::json; use std::time::{Duration, Instant}; + #[tokio::test] + async fn enqueue_url_validation_blocks_local_http_but_preserves_ftp() { + assert_eq!( + validate_enqueue_url("http://127.0.0.1/file.zip").await, + Err("SSRF blocked: Private/local IP not allowed".to_string()) + ); + assert_eq!( + validate_enqueue_url("ftp://127.0.0.1/file.zip").await, + Err("SSRF blocked: Private/local IP not allowed".to_string()) + ); + assert_eq!( + validate_enqueue_url("sftp://[::1]/file.zip").await, + Err("SSRF blocked: Private/local IP not allowed".to_string()) + ); + assert_eq!( + validate_enqueue_url("http://[::ffff:127.0.0.1]/file.zip").await, + Err("SSRF blocked: Private/local IP not allowed".to_string()) + ); + assert_eq!( + validate_enqueue_url("file:///tmp/file.zip").await, + Err("Unsupported URL scheme".to_string()) + ); + } + #[test] fn slow_nonzero_aria2_throughput_recovers_after_a_sustained_degradation() { let start = Instant::now(); diff --git a/src/components/AddDownloadsModal.tsx b/src/components/AddDownloadsModal.tsx index 3e80479..a46a63b 100644 --- a/src/components/AddDownloadsModal.tsx +++ b/src/components/AddDownloadsModal.tsx @@ -92,7 +92,8 @@ export const AddDownloadsModal = () => { baseDownloadFolder, perServerConnections, keychainAccessReady, - keychainPromptDismissed + keychainPromptDismissed, + showKeychainModal } = useSettingsStore(); const [urls, setUrls] = useState(''); @@ -168,13 +169,13 @@ export const AddDownloadsModal = () => { row.playlistSourceUrl || row.sourceUrl; const closeModalFromDismissAction = useCallback(() => { - if (isSubmitting || isSubmittingRef.current) return; + if (isSubmitting || isSubmittingRef.current || showKeychainModal) return; const hasPendingInput = Boolean( urls.trim() || pendingAddUrls.trim() || parsedItems.length || headers.trim() || cookies.trim() ); if (hasPendingInput && !window.confirm('Discard this download setup?')) return; toggleAddModal(false); - }, [cookies, headers, isSubmitting, parsedItems.length, pendingAddUrls, toggleAddModal, urls]); + }, [cookies, headers, isSubmitting, parsedItems.length, pendingAddUrls, showKeychainModal, toggleAddModal, urls]); useEffect(() => { if (!isAddModalOpen) { @@ -271,6 +272,7 @@ export const AddDownloadsModal = () => { if (!isAddModalOpen) return; const closeOnEscape = (event: KeyboardEvent) => { if (event.key !== 'Escape') return; + if (showKeychainModal) return; if (showingDuplicates) { setShowingDuplicates(false); } else if (isQueueMenuOpen) { @@ -281,7 +283,7 @@ export const AddDownloadsModal = () => { }; window.addEventListener('keydown', closeOnEscape); return () => window.removeEventListener('keydown', closeOnEscape); - }, [closeModalFromDismissAction, isAddModalOpen, isQueueMenuOpen, showingDuplicates]); + }, [closeModalFromDismissAction, isAddModalOpen, isQueueMenuOpen, showKeychainModal, showingDuplicates]); useEffect(() => { const requestId = ++freeSpaceRequestRef.current; @@ -511,12 +513,23 @@ export const AddDownloadsModal = () => { size: meta.size_bytes ? meta.size : undefined, sizeBytes: meta.size_bytes || undefined, status: 'ready', - resumable: meta.resumable + resumable: meta.resumable, + metadataBlockedReason: undefined }) )); } } catch (e) { console.error("Meta fetch failed", e); + const errorMessage = e instanceof Error ? e.message : String(e); + const metadataBlockedReason = [ + 'SSRF blocked: Invalid URL', + 'SSRF blocked: No host', + 'SSRF blocked: DNS resolution failed', + 'SSRF blocked: No DNS records', + 'SSRF blocked: Private/local IP not allowed' + ].some(prefix => errorMessage.startsWith(prefix)) + ? 'unsafe-url' as const + : undefined; setParsedItems(current => updateRowIfCurrent( current, row.id, @@ -530,8 +543,9 @@ export const AddDownloadsModal = () => { status: 'metadata-error', formats: undefined, selectedFormat: undefined, + metadataBlockedReason, playlistError: row.isPlaylist - ? (e instanceof Error ? e.message : String(e)) + ? errorMessage : undefined }) )); @@ -1003,7 +1017,10 @@ export const AddDownloadsModal = () => { const failedMediaMetadataCount = selectedItems.filter( item => item.status === 'metadata-error' && item.isMedia ).length; - const fallbackMetadataCount = failedMetadataCount - failedMediaMetadataCount; + const blockedMetadataCount = selectedItems.filter( + item => item.metadataBlockedReason === 'unsafe-url' + ).length; + const fallbackMetadataCount = failedMetadataCount - failedMediaMetadataCount - blockedMetadataCount; const activePlaylistUrls = new Set( urls.split('\n').map(url => url.trim()).filter(Boolean).map(normalizeComparableUrl) ); @@ -1084,7 +1101,7 @@ export const AddDownloadsModal = () => { })}
- {selectedItems.filter(item => item.status === 'ready').length} selected ready, {fallbackMetadataCount} fallback, {failedMediaMetadataCount} media retry + {selectedItems.filter(item => item.status === 'ready').length} selected ready, {fallbackMetadataCount} fallback, {failedMediaMetadataCount} media retry, {blockedMetadataCount} blocked
) : ( item.status === 'metadata-error' - ? item.isPlaylist ? 'Playlist failed' : item.isMedia ? 'Metadata failed' : 'Fallback' + ? item.metadataBlockedReason === 'unsafe-url' ? 'Unsafe URL' : item.isPlaylist ? 'Playlist failed' : item.isMedia ? 'Metadata failed' : 'Fallback' : item.status === 'invalid' ? 'Invalid' : 'Ready' @@ -1404,7 +1421,7 @@ export const AddDownloadsModal = () => { {metadataSummaryMessage(parsedItems)}
-
diff --git a/src/components/KeychainPermissionModal.tsx b/src/components/KeychainPermissionModal.tsx index 4be94cd..d0bcb1d 100644 --- a/src/components/KeychainPermissionModal.tsx +++ b/src/components/KeychainPermissionModal.tsx @@ -5,6 +5,9 @@ import { KeyRound, ShieldAlert } from 'lucide-react'; import { usePlatformInfo } from '../utils/platform'; import { getKeychainConsentVersion } from '../utils/keychainStartup'; import { getVersion } from '@tauri-apps/api/app'; +import type { PairingTokenHydration } from '../bindings/PairingTokenHydration'; + +const KEYCHAIN_GRANT_TIMEOUT_MS = 30_000; type KeychainPermissionModalProps = { consentVersion: string; @@ -54,27 +57,48 @@ export const KeychainPermissionModal: React.FC = ( setIsGranting(true); setError(null); + let timeoutId: number | undefined; + let persistentGrantApplied = false; + const applyPersistentGrant = async (result: PairingTokenHydration): Promise => { + if (!result.persistent || persistentGrantApplied) return result.persistent; + persistentGrantApplied = true; + const grantedVersion = consentVersion || getKeychainConsentVersion(await getVersion().catch(() => '')); + // Keep state in sync with the grant result instead of rehydrating + // before Zustand has persisted keychainAccessGranted. + useSettingsStore.setState({ + keychainAccessGranted: true, + keychainAccessVersion: grantedVersion, + keychainAccessReady: true, + extensionPairingToken: result.token, + isPairingTokenPersistent: true, + keychainPromptDismissed: false, + showKeychainModal: false + }); + return true; + }; + const grantRequest = invoke('grant_keychain_access'); + // A native credential-store call cannot be cancelled by the webview. Keep + // a late successful result useful even if the UI timeout has already + // restored the Later/retry controls. + grantRequest.then(applyPersistentGrant).catch(() => undefined); + try { - const result = await invoke('grant_keychain_access'); - if (result.persistent) { - const grantedVersion = consentVersion || getKeychainConsentVersion(await getVersion().catch(() => '')); - // Keep state in sync with the grant result instead of rehydrating - // before Zustand has persisted keychainAccessGranted. - useSettingsStore.setState({ - keychainAccessGranted: true, - keychainAccessVersion: grantedVersion, - keychainAccessReady: true, - extensionPairingToken: result.token, - isPairingTokenPersistent: true, - keychainPromptDismissed: false, - showKeychainModal: false - }); - } else { + const result = await Promise.race([ + grantRequest, + new Promise((_, reject) => { + timeoutId = window.setTimeout( + () => reject(new Error('Credential storage request timed out. You can select Later and try again.')), + KEYCHAIN_GRANT_TIMEOUT_MS + ); + }) + ]); + if (!(await applyPersistentGrant(result))) { setError(result.error || `${siteCredentialStoreName} is unavailable.`); } } catch (e: any) { setError(e.toString()); } finally { + if (timeoutId !== undefined) window.clearTimeout(timeoutId); setIsGranting(false); } }; diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index 3e314ff..28a29ed 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -38,6 +38,8 @@ export const Sidebar: React.FC = (props) => { const addInputRef = useRef(null); const renameInputRef = useRef(null); + const addQueueSubmitRef = useRef(false); + const renameQueueSubmitRef = useRef(false); useEffect(() => { const handleCloseMenu = () => setContextMenu(null); @@ -139,15 +141,34 @@ export const Sidebar: React.FC = (props) => { }; const handleAddQueueSubmit = () => { - if (newQueueName.trim()) addQueue(newQueueName.trim()); + if (addQueueSubmitRef.current) return; + const normalizedName = newQueueName.trim(); + if (!normalizedName) { + addToast({ message: 'Queue name cannot be empty', variant: 'error', isActionable: true }); + return; + } + if (!addQueue(normalizedName)) { + addToast({ message: 'A queue with this name already exists', variant: 'error', isActionable: true }); + return; + } + addQueueSubmitRef.current = true; setNewQueueName(''); setIsAddingQueue(false); }; const handleRenameQueueSubmit = () => { - if (renamingQueueId && editingQueueName.trim()) { - renameQueue(renamingQueueId, editingQueueName.trim()); + if (renameQueueSubmitRef.current) return; + const normalizedName = editingQueueName.trim(); + if (!renamingQueueId) return; + if (!normalizedName) { + addToast({ message: 'Queue name cannot be empty', variant: 'error', isActionable: true }); + return; } + if (!renameQueue(renamingQueueId, normalizedName)) { + addToast({ message: 'A queue with this name already exists', variant: 'error', isActionable: true }); + return; + } + renameQueueSubmitRef.current = true; setRenamingQueueId(null); }; @@ -293,7 +314,7 @@ export const Sidebar: React.FC = (props) => { ) : (