diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a2a995e..e6e8b50 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,8 @@ **Vulnerability:** The application defaulted to displaying the first available resource when a requested resource was not found in the allowed list. This allowed an external configuration file to display a misleading caption (from the file) while showing an unrelated valid image/video (from the fallback), creating a spoofing vulnerability. **Learning:** "Last resort" or "Fail Open" logic in resource loaders can compromise data integrity and user trust. Always "Fail Closed" (show nothing or an error) when a specific resource is not found. **Prevention:** Ensure that lookup functions return an error code or null when a match is not found, rather than returning a default value that could be misinterpreted as the requested item. + +## 2024-10-25 - Enforce HTTPS for External Content +**Vulnerability:** Loading content via cleartext HTTP allows for potential Man-in-the-Middle (MITM) attacks and exposes user viewing habits (privacy leak). +**Learning:** Even in game engines like Unity/VRChat, ensuring transport security is critical when fetching external resources. +**Prevention:** Enforce HTTPS upgrade logic in URL parsers. Automatically upgrade `http:` schemes to `https:` or reject them. diff --git a/Scripts/ImageLoader.cs b/Scripts/ImageLoader.cs index 8bccf34..6cbe9ac 100644 --- a/Scripts/ImageLoader.cs +++ b/Scripts/ImageLoader.cs @@ -288,6 +288,18 @@ public override void OnStringLoadSuccess(IVRCStringDownload result) int matchingUrlIndex = FindMatchingUrlIndex(urlStr, tempCount); if (matchingUrlIndex >= 0) { + // Security: If we matched an HTTP URL but have an HTTPS replacement, update the predefined URL + // This ensures we actually load over HTTPS + if (predefinedUrls[matchingUrlIndex] != null) + { + string predefUrl = predefinedUrls[matchingUrlIndex].Get(); + if (predefUrl.StartsWith("http:") && urlStr.StartsWith("https:")) + { + Debug.LogWarning($"[Security] Hot-swapping predefined URL at index {matchingUrlIndex} to HTTPS to ensure secure loading."); + predefinedUrls[matchingUrlIndex] = new VRCUrl(urlStr); + } + } + // Check if this URL index is already in our active URLs bool alreadyActive = false; for (int i = 0; i < _activeUrlIndices.Length; i++) @@ -360,14 +372,16 @@ public override void OnStringLoadSuccess(IVRCStringDownload result) private string ExtractUrlFromLine(string line) { + string extractedUrl = ""; + // Format 1: "URL" (direct URL) if (line.StartsWith("http")) { - return line; + extractedUrl = line; } // Format 2: "filename.png: URL" (colon separated) - if (line.Contains(":")) + if (string.IsNullOrEmpty(extractedUrl) && line.Contains(":")) { int colonPos = line.IndexOf(":"); if (colonPos >= 0 && colonPos < line.Length - 1) @@ -375,19 +389,30 @@ private string ExtractUrlFromLine(string line) string afterColon = line.Substring(colonPos + 1).Trim(); if (afterColon.StartsWith("http")) { - return afterColon; + extractedUrl = afterColon; } } } // Format 3: "n. filename.png: URL" (numbered list) - int httpIndex = line.IndexOf("http"); - if (httpIndex >= 0) + if (string.IsNullOrEmpty(extractedUrl)) { - return line.Substring(httpIndex).Trim(); + int httpIndex = line.IndexOf("http"); + if (httpIndex >= 0) + { + extractedUrl = line.Substring(httpIndex).Trim(); + } + } + + // Security enhancement: Enforce HTTPS + // If we found a URL but it's using insecure HTTP, upgrade it + if (!string.IsNullOrEmpty(extractedUrl) && extractedUrl.StartsWith("http:")) + { + Debug.LogWarning($"[Security] Upgrading insecure HTTP URL to HTTPS: {extractedUrl}"); + extractedUrl = "https:" + extractedUrl.Substring(5); } - return ""; + return extractedUrl; } private string ExtractCaptionFromLine(string line) @@ -428,12 +453,26 @@ private int FindMatchingUrlIndex(string urlToFind, int additionalCount = 0) return -1; } - // First try to find an exact match + // First try to find an exact match or an HTTPS upgrade match for (int i = 0; i < predefinedUrls.Length; i++) { - if (predefinedUrls[i] != null && predefinedUrls[i].Get() == urlToFind) + if (predefinedUrls[i] != null) { - return i; + string predefUrl = predefinedUrls[i].Get(); + + // Exact match + if (predefUrl == urlToFind) + { + return i; + } + + // Check if upgrading the predefined URL to HTTPS matches the input (which is already HTTPS) + if (predefUrl.StartsWith("http:") && + urlToFind.StartsWith("https:") && + predefUrl.Substring(4) == urlToFind.Substring(5)) + { + return i; + } } } @@ -603,11 +642,25 @@ public override void OnImageLoadSuccess(IVRCImageDownload result) for (int i = 0; i < _activeUrlIndices.Length; i++) { int urlIndex = _activeUrlIndices[i]; - if (urlIndex < predefinedUrls.Length && predefinedUrls[urlIndex] != null && - predefinedUrls[urlIndex].Get() == loadedUrl) + if (urlIndex < predefinedUrls.Length && predefinedUrls[urlIndex] != null) { - // Store the downloaded texture - _downloadedTextures[i] = result.Result; + string predefUrl = predefinedUrls[urlIndex].Get(); + bool isMatch = (predefUrl == loadedUrl); + + // If not exact match, check if it's an HTTP vs HTTPS mismatch due to hot-swap + // loadedUrl might be http (original download request) while predefinedUrl is now https (hot-swapped) + if (!isMatch && + loadedUrl.StartsWith("http:") && + predefUrl.StartsWith("https:") && + loadedUrl.Substring(4) == predefUrl.Substring(5)) + { + isMatch = true; + } + + if (isMatch) + { + // Store the downloaded texture + _downloadedTextures[i] = result.Result; Debug.Log($"Stored downloaded texture at index {i}"); // If this is the currently displayed image, update the display diff --git a/Scripts/VideoURLProvider.cs b/Scripts/VideoURLProvider.cs index 8a7504b..9fd1598 100644 --- a/Scripts/VideoURLProvider.cs +++ b/Scripts/VideoURLProvider.cs @@ -922,6 +922,18 @@ public override void OnStringLoadSuccess(IVRCStringDownload result) // Check the return value appropriately if (matchingUrlIndex >= 0) { + // Security: If we matched an HTTP URL but have an HTTPS replacement, update the predefined URL + // This ensures we actually load over HTTPS + if (predefinedUrls[matchingUrlIndex] != null) + { + string predefUrl = predefinedUrls[matchingUrlIndex].Get(); + if (predefUrl.StartsWith("http:") && urlStr.StartsWith("https:")) + { + Debug.LogWarning($"[Security] Hot-swapping predefined URL at index {matchingUrlIndex} to HTTPS to ensure secure loading."); + predefinedUrls[matchingUrlIndex] = new VRCUrl(urlStr); + } + } + // Extract caption if available string caption = ExtractCaptionFromLine(trimmedLine); if (string.IsNullOrEmpty(caption)) @@ -983,20 +995,34 @@ private int FindMatchingUrlIndex(string urlToFind) // First try to find an exact match in predefined URLs for (int i = 0; i < predefinedUrls.Length; i++) { - if (predefinedUrls[i] != null && predefinedUrls[i].Get() == urlToFind) + if (predefinedUrls[i] != null) { - // Check if this URL index is already in our active indices - for (int j = 0; j < _activeUrlIndices.Length; j++) + string predefUrl = predefinedUrls[i].Get(); + bool isMatch = (predefUrl == urlToFind); + + // Check if upgrading the predefined URL to HTTPS matches the input (which is already HTTPS) + if (!isMatch && predefUrl.StartsWith("http:") && + urlToFind.StartsWith("https:") && + predefUrl.Substring(4) == urlToFind.Substring(5)) + { + isMatch = true; + } + + if (isMatch) { - if (_activeUrlIndices[j] == i) + // Check if this URL index is already in our active indices + for (int j = 0; j < _activeUrlIndices.Length; j++) { - Debug.Log($"[VideoURLProvider] URL already exists in playlist at index {j}, skipping duplicate: {urlToFind}"); - return -1; // Return -1 to indicate we should skip adding this duplicate + if (_activeUrlIndices[j] == i) + { + Debug.Log($"[VideoURLProvider] URL already exists in playlist at index {j}, skipping duplicate: {urlToFind}"); + return -1; // Return -1 to indicate we should skip adding this duplicate + } } + + // If not already in playlist, return this index + return i; } - - // If not already in playlist, return this index - return i; } } @@ -1093,14 +1119,16 @@ private void TrimOldestUrls(int countToRemove) private string ExtractUrlFromLine(string line) { + string extractedUrl = ""; + // Direct URL format if (line.StartsWith("http")) { - return line; + extractedUrl = line; } // Title: URL format - if (line.Contains(":")) + if (string.IsNullOrEmpty(extractedUrl) && line.Contains(":")) { int colonPos = line.IndexOf(":"); if (colonPos >= 0 && colonPos < line.Length - 1) @@ -1108,22 +1136,33 @@ private string ExtractUrlFromLine(string line) string afterColon = line.Substring(colonPos + 1).Trim(); if (afterColon.StartsWith("http")) { - return afterColon; + extractedUrl = afterColon; } } } // Find any URL in the line - int httpIndex = line.IndexOf("http"); - if (httpIndex >= 0) + if (string.IsNullOrEmpty(extractedUrl)) { - string substr = line.Substring(httpIndex); - // Attempt to find the end of the URL by looking for whitespace - int spaceIndex = substr.IndexOf(' '); - return spaceIndex > 0 ? substr.Substring(0, spaceIndex) : substr; + int httpIndex = line.IndexOf("http"); + if (httpIndex >= 0) + { + string substr = line.Substring(httpIndex); + // Attempt to find the end of the URL by looking for whitespace + int spaceIndex = substr.IndexOf(' '); + extractedUrl = spaceIndex > 0 ? substr.Substring(0, spaceIndex) : substr; + } + } + + // Security enhancement: Enforce HTTPS + // If we found a URL but it's using insecure HTTP, upgrade it + if (!string.IsNullOrEmpty(extractedUrl) && extractedUrl.StartsWith("http:")) + { + Debug.LogWarning($"[Security] Upgrading insecure HTTP URL to HTTPS: {extractedUrl}"); + extractedUrl = "https:" + extractedUrl.Substring(5); } - return ""; + return extractedUrl; } private string ExtractCaptionFromLine(string line)