diff --git a/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.Params.cs b/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.Params.cs index a57ed909f85..a4ac3281630 100644 --- a/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.Params.cs +++ b/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.Params.cs @@ -1,4 +1,6 @@ -ο»Ώnamespace DCL.ChangeRealmPrompt +using UnityEngine; + +namespace DCL.ChangeRealmPrompt { public partial class ChangeRealmPromptController { @@ -7,10 +9,14 @@ public struct Params public string Message { get; } public string Realm { get; } - public Params(string message, string realm) + /// Optional target parcel to land on after the realm switch. + public Vector2Int? Position { get; } + + public Params(string message, string realm, Vector2Int? position = null) { Message = message; Realm = realm; + Position = position; } } } diff --git a/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.cs b/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.cs index 28ed665798e..1eb81ef5c17 100644 --- a/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.cs +++ b/Explorer/Assets/DCL/ChangeRealmPrompt/ChangeRealmPromptController.cs @@ -3,6 +3,7 @@ using MVC; using System; using System.Threading; +using UnityEngine; namespace DCL.ChangeRealmPrompt { @@ -10,14 +11,16 @@ public partial class ChangeRealmPromptController : ControllerBase CanvasOrdering.SortingLayer.POPUP; + private const string DEFAULT_CONFIRMATION_MESSAGE = "Are you sure you want to enter this World?"; + private readonly ICursor cursor; - private readonly Action changeRealmCallback; - private Action resultCallback; + private readonly Action changeRealmCallback; + private Action? resultCallback; public ChangeRealmPromptController( ViewFactoryMethod viewFactory, ICursor cursor, - Action changeRealmCallback) : base(viewFactory) + Action changeRealmCallback) : base(viewFactory) { this.cursor = cursor; this.changeRealmCallback = changeRealmCallback; @@ -25,9 +28,14 @@ public ChangeRealmPromptController( protected override void OnViewInstantiated() { - viewInstance.CloseButton.onClick.AddListener(Dismiss); + viewInstance!.CloseButton.onClick.AddListener(Dismiss); viewInstance.CancelButton.onClick.AddListener(Dismiss); viewInstance.ContinueButton.onClick.AddListener(Approve); + + // Message and realm are attacker-controllable (scene changeRealm / deep link). Disable rich-text + // parsing so neither can inject TMP markup into this consent prompt (SEC-003; same class as SEC-034/050). + viewInstance.MessageText.richText = false; + viewInstance.RealmText.richText = false; } protected override void OnViewShow() @@ -38,32 +46,50 @@ protected override void OnViewShow() if (result != ChangeRealmPromptResultType.Approved) return; - changeRealmCallback?.Invoke(inputData.Realm); + changeRealmCallback.Invoke(inputData.Realm, inputData.Position); }); } - protected override UniTask WaitForCloseIntentAsync(CancellationToken ct) - { - if (string.IsNullOrEmpty(inputData.Message)) - return UniTask.CompletedTask; - - return UniTask.WhenAny( - viewInstance.CloseButton.OnClickAsync(ct), + protected override UniTask WaitForCloseIntentAsync(CancellationToken ct) => + UniTask.WhenAny( + viewInstance!.CloseButton.OnClickAsync(ct), viewInstance.CancelButton.OnClickAsync(ct), viewInstance.ContinueButton.OnClickAsync(ct)); - } private void RequestChangeRealm(string message, string realm, Action result) { resultCallback = result; + viewInstance!.MessageText.text = string.IsNullOrEmpty(message) ? DEFAULT_CONFIRMATION_MESSAGE : message; + viewInstance.RealmText.text = DestinationHostFor(realm); + } - if (string.IsNullOrEmpty(message)) - resultCallback?.Invoke(ChangeRealmPromptResultType.Approved); - else + /// + /// The destination shown to the user: for a URL realm the authority (host[:port]) with any misleading + /// userinfo (https://trusted@evil.com) and path/query stripped, so the true host is shown β€” not a + /// spoof; a world name or realm alias is shown unchanged. This β€” with rich-text disabled in + /// β€” is what the user actually consents to. + /// + internal static string DestinationHostFor(string realm) + { + int schemeIdx = realm.IndexOf("://", StringComparison.Ordinal); + + if (schemeIdx < 0) + return realm; + + int start = schemeIdx + 3; + int end = start; + + while (end < realm.Length && realm[end] != '/' && realm[end] != '?' && realm[end] != '#') { - viewInstance.MessageText.text = message; - viewInstance.RealmText.text = realm; + // Skip past any userinfo (e.g. https://decentraland.org@evil.com) so the real host after the + // last '@' is displayed, not the trusted-looking prefix (consent-prompt spoofing, SEC-004). + if (realm[end] == '@') + start = end + 1; + + end++; } + + return end > start ? realm.Substring(start, end - start) : realm; } private void Dismiss() => diff --git a/Explorer/Assets/DCL/Chat/Commands/ChatEnvironmentValidator.cs b/Explorer/Assets/DCL/Chat/Commands/ChatEnvironmentValidator.cs index b3db2a61d9e..323441ec952 100644 --- a/Explorer/Assets/DCL/Chat/Commands/ChatEnvironmentValidator.cs +++ b/Explorer/Assets/DCL/Chat/Commands/ChatEnvironmentValidator.cs @@ -1,21 +1,19 @@ using DCL.Multiplayer.Connections.DecentralandUrls; using DCL.Utility.Types; -using UnityEngine.Networking; +using System; namespace DCL.Chat.Commands { public class ChatEnvironmentValidator { - private readonly DecentralandEnvironment dclEnvironment; + private const string ORG_HOST_SUFFIX = "decentraland.org"; + private const string ZONE_HOST_SUFFIX = "decentraland.zone"; - private readonly string zoneDescription; - private readonly string orgDescription; + private readonly DecentralandEnvironment dclEnvironment; public ChatEnvironmentValidator(DecentralandEnvironment dclEnvironment) { this.dclEnvironment = dclEnvironment; - zoneDescription = DecentralandEnvironment.Zone.ToString().ToLower(); - orgDescription = DecentralandEnvironment.Org.ToString().ToLower(); } public Result ValidateTeleport(string realmToTeleportTo) @@ -26,19 +24,63 @@ public Result ValidateTeleport(string realmToTeleportTo) return Result.ErrorResult( "πŸ”΄ Error. You cannot change realms in the Today environment. Please restart DCL with the desired environment"); case DecentralandEnvironment.Zone: - if (realmToTeleportTo.Contains(zoneDescription)) - return Result.SuccessResult(); - return Result.ErrorResult( - "πŸ”΄ Error. You cannot teleport to other realms that are not Zone in Zone environment. Please restart DCL with the desired environment"); + return HostHasSuffix(realmToTeleportTo, ZONE_HOST_SUFFIX) + ? Result.SuccessResult() + : Result.ErrorResult( + "πŸ”΄ Error. You cannot teleport to other realms that are not Zone in Zone environment. Please restart DCL with the desired environment"); case DecentralandEnvironment.Org: - if (realmToTeleportTo.Contains(orgDescription)) - return Result.SuccessResult(); - - return Result.ErrorResult( - "πŸ”΄ Error. You cannot teleport to other realms that are not Org or World in Org environment. Please restart DCL with the desired environment"); + return HostHasSuffix(realmToTeleportTo, ORG_HOST_SUFFIX) + ? Result.SuccessResult() + : Result.ErrorResult( + "πŸ”΄ Error. You cannot teleport to other realms that are not Org or World in Org environment. Please restart DCL with the desired environment"); } return Result.SuccessResult(); } + + /// + /// True if the URL's host equals or ends with "." + suffix at a domain + /// boundary. Reads the host out of the string via a span (no allocation); a URL + /// carrying userinfo ('@') is rejected so it cannot spoof the host β€” the '@' is checked before any ':' + /// so a colon-before-'@' form (https://decentraland.org:1234@evil.com) cannot slip past the guard. + /// + private static bool HostHasSuffix(string url, string suffix) + { + int schemeIdx = url.IndexOf("://", StringComparison.Ordinal); + + if (schemeIdx < 0) + return false; + + int hostStart = schemeIdx + 3; + int authorityEnd = hostStart; + + while (authorityEnd < url.Length) + { + char c = url[authorityEnd]; + + if (c == '/' || c == '?' || c == '#') + break; + + // userinfo is not expected in a realm URL; reject rather than risk host-confusion. Checked BEFORE + // any ':' so https://decentraland.org:1234@evil.com (port before userinfo) can't slip past. + if (c == '@') + return false; + + authorityEnd++; + } + + // Strip the port (if any) from the authority to get the bare host. + ReadOnlySpan authority = url.AsSpan(hostStart, authorityEnd - hostStart); + int portSep = authority.IndexOf(':'); + ReadOnlySpan host = portSep >= 0 ? authority.Slice(0, portSep) : authority; + + if (host.Equals(suffix, StringComparison.OrdinalIgnoreCase)) + return true; + + // subdomain boundary match: host ends with ".{suffix}" + return host.Length > suffix.Length + && host[host.Length - suffix.Length - 1] == '.' + && host.EndsWith(suffix, StringComparison.OrdinalIgnoreCase); + } } } diff --git a/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.Params.cs b/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.Params.cs index ef8af24a015..46a4baf0257 100644 --- a/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.Params.cs +++ b/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.Params.cs @@ -1,4 +1,5 @@ -ο»Ώusing System; +ο»Ώusing DCL.Browser; +using System; namespace DCL.ExternalUrlPrompt { @@ -6,11 +7,14 @@ public partial class ExternalUrlPromptController { public struct Params { - public Uri Uri { get; } + /// Null when url is not an absolute http/https URL β€” callers must null-check. + public Uri? Uri { get; } public Params(string url) { - Uri = Uri.TryCreate(url, UriKind.Absolute, out Uri uri) ? uri : null; + Uri = ExternalUrlPolicy.IsWebScheme(url) && Uri.TryCreate(url, UriKind.Absolute, out Uri uri) + ? uri + : null; } } } diff --git a/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.cs b/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.cs index c2b9f1304b2..3a4e59c9b1a 100644 --- a/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.cs +++ b/Explorer/Assets/DCL/ExternalUrlPrompt/ExternalUrlPromptController.cs @@ -14,8 +14,8 @@ public partial class ExternalUrlPromptController : ControllerBase trustedDomains = new (); - private Action resultCallback; + private readonly HashSet trustedKeys = new (); + private Action? resultCallback; public ExternalUrlPromptController( ViewFactoryMethod viewFactory, @@ -28,7 +28,7 @@ public ExternalUrlPromptController( protected override void OnViewInstantiated() { - viewInstance.CloseButton.onClick.AddListener(Dismiss); + viewInstance!.CloseButton.onClick.AddListener(Dismiss); viewInstance.CancelButton.onClick.AddListener(Dismiss); viewInstance.ContinueButton.onClick.AddListener(Approve); } @@ -38,25 +38,28 @@ protected override void OnViewShow() if (inputData.Uri == null) return; - if (trustedDomains.Contains(inputData.Uri.Host)) + Uri uri = inputData.Uri; + + if (ExternalUrlPolicy.TryGetTrustKey(uri, out string trustKey) && trustedKeys.Contains(trustKey)) { - webBrowser.OpenUrlMainThreadOnly(inputData.Uri.OriginalString); - viewInstance.CloseButton.OnClickAsync(CancellationToken.None).Forget(); + webBrowser.OpenUrlMainThreadOnly(uri.OriginalString); + viewInstance!.CloseButton.OnClickAsync(CancellationToken.None).Forget(); return; } cursor.Unlock(); - RequestOpenUrl(inputData.Uri, result => + RequestOpenUrl(uri, result => { switch (result) { case ExternalUrlPromptResultType.ApprovedTrusted: - if (!trustedDomains.Contains(inputData.Uri.Host)) - trustedDomains.Add(inputData.Uri.Host); - webBrowser.OpenUrlMainThreadOnly(inputData.Uri.OriginalString); + // Only cache when a real (scheme, host) key exists β€” empty-host URIs are never trusted (SEC-008). + if (ExternalUrlPolicy.TryGetTrustKey(uri, out string key)) + trustedKeys.Add(key); + webBrowser.OpenUrlMainThreadOnly(uri.OriginalString); break; case ExternalUrlPromptResultType.Approved: - webBrowser.OpenUrlMainThreadOnly(inputData.Uri.OriginalString); + webBrowser.OpenUrlMainThreadOnly(uri.OriginalString); break; } }); @@ -64,7 +67,9 @@ protected override void OnViewShow() protected override UniTask WaitForCloseIntentAsync(CancellationToken ct) { - if (inputData.Uri != null && trustedDomains.Contains(inputData.Uri.Host)) + if (inputData.Uri != null + && ExternalUrlPolicy.TryGetTrustKey(inputData.Uri, out string trustKey) + && trustedKeys.Contains(trustKey)) return UniTask.CompletedTask; return UniTask.WhenAny( @@ -75,13 +80,13 @@ protected override UniTask WaitForCloseIntentAsync(CancellationToken ct) public override void Dispose() { - trustedDomains.Clear(); + trustedKeys.Clear(); } private void RequestOpenUrl(Uri uri, Action result) { resultCallback = result; - viewInstance.DomainText.text = uri.Host; + viewInstance!.DomainText.text = uri.Host; viewInstance.UrlText.text = uri.OriginalString; viewInstance.TrustToggle.isOn = false; } @@ -90,6 +95,6 @@ private void Dismiss() => resultCallback?.Invoke(ExternalUrlPromptResultType.Canceled); private void Approve() => - resultCallback?.Invoke(viewInstance.TrustToggle.isOn ? ExternalUrlPromptResultType.ApprovedTrusted : ExternalUrlPromptResultType.Approved); + resultCallback?.Invoke(viewInstance!.TrustToggle.isOn ? ExternalUrlPromptResultType.ApprovedTrusted : ExternalUrlPromptResultType.Approved); } } diff --git a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Cache/GltfContainerAssetsCache.cs b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Cache/GltfContainerAssetsCache.cs index 30b9b80ee97..6c0cc3343fd 100644 --- a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Cache/GltfContainerAssetsCache.cs +++ b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Cache/GltfContainerAssetsCache.cs @@ -90,6 +90,12 @@ public bool TryGet(in string key, out GltfContainerAsset? asset) /// public void Dereference(in string key, GltfContainerAsset asset, bool putInBridge = false, bool handleAssetLoad = true) { + // A stale promise result can still reference an asset whose Root was already destroyed + // (e.g. drained by Unload). Comparing against Unity's overloaded null catches that destroyed + // GameObject; re-pooling it would crash DereferenceFinalOperation and hand a dead instance back out. + if (asset.Root == null) + return; + if (handleAssetLoad && assetLoadCache != null && assetLoadCache.ContainsGltf(key)) { assetLoadCache.ReleaseGltfInstance(key, asset); diff --git a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Tests/GltfContainerAssetsCacheShould.cs b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Tests/GltfContainerAssetsCacheShould.cs new file mode 100644 index 00000000000..b0d9ee4994d --- /dev/null +++ b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Tests/GltfContainerAssetsCacheShould.cs @@ -0,0 +1,48 @@ +using DCL.Optimization.Pools; +using ECS.Unity.GLTFContainer.Asset.Cache; +using ECS.Unity.GLTFContainer.Asset.Components; +using NSubstitute; +using NUnit.Framework; +using UnityEngine; +using Object = UnityEngine.Object; + +namespace ECS.Unity.GLTFContainer.Asset.Tests +{ + [TestFixture] + public class GltfContainerAssetsCacheShould + { + private GltfContainerAssetsCache cache; + + [SetUp] + public void SetUp() + { + cache = new GltfContainerAssetsCache(Substitute.For()); + } + + [Test] + public void PoolLiveAssetOnDereference() + { + var asset = GltfContainerAsset.Create(new GameObject(), assetData: null); + + cache.Dereference("hash", asset); + + Assert.That(cache.TryGet("hash", out GltfContainerAsset? pooled), Is.True); + Assert.That(pooled, Is.SameAs(asset)); + + Object.DestroyImmediate(asset.Root); + } + + [Test] + public void SkipDereferenceWhenRootAlreadyDestroyed() + { + // A stale promise result can reference an asset whose Root was destroyed by a prior Unload. + var asset = GltfContainerAsset.Create(new GameObject(), assetData: null); + Object.DestroyImmediate(asset.Root); + + Assert.DoesNotThrow(() => cache.Dereference("hash", asset)); + + // The dead asset must not be re-pooled, otherwise TryGet would hand a destroyed instance back out. + Assert.That(cache.TryGet("hash", out _), Is.False); + } + } +} diff --git a/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Tests/GltfContainerAssetsCacheShould.cs.meta b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Tests/GltfContainerAssetsCacheShould.cs.meta new file mode 100644 index 00000000000..54562a9af33 --- /dev/null +++ b/Explorer/Assets/DCL/Infrastructure/ECS/Unity/GLTFContainer/Asset/Tests/GltfContainerAssetsCacheShould.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 21b8cd3964c9b8642b70e255e64b11b4 \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs index 3fadae5145b..d332d9bdc23 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/AppArgsFlags.cs @@ -64,7 +64,6 @@ public static class AppArgsFlags public const string SIMULATE_MEMORY = "simulateMemory"; public const string LAUNCH_CDP_MONITOR_ON_START = "launch-cdp-monitor-on-start"; - public const string CREATOR_HUB_BIN_PATH = "creator-hub-bin-path"; public const string USE_LOG_MATRIX = "use-log-matrix"; public const string GRAPHICS = "graphics"; diff --git a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/ApplicationParametersParser.cs b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/ApplicationParametersParser.cs index a2715257e4f..ae6348775b7 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/ApplicationParametersParser.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/ApplicationParametersParser.cs @@ -101,11 +101,16 @@ public static Dictionary ProcessDeepLinkParameters(string deepLi var uri = new Uri(deepLinkString); NameValueCollection uriQuery = HttpUtility.ParseQueryString(uri.Query); + var droppedKeys = new List(); + + // Tier 1: always-permitted (base) navigation/login params. foreach (string uriQueryKey in uriQuery.AllKeys) { // if the deep link is not constructed correctly (AKA 'decentraland://?&blabla=blabla') a 'null' parameter can be detected... if (uriQueryKey == null) continue; - output[uriQueryKey] = uriQuery.Get(uriQueryKey); + + if (DeepLinkAllowlist.IsPermitted(uriQueryKey)) + output[uriQueryKey] = uriQuery.Get(uriQueryKey); } if (output.TryGetValue(AppArgsFlags.REALM, out string? realmParamValue)) @@ -120,9 +125,26 @@ public static Dictionary ProcessDeepLinkParameters(string deepLi output[AppArgsFlags.REALM] = realmParamValue; } - // Patch for WinOS sometimes affecting the 'skip-auth-screen' parameter in deep links putting a '/' at the end - if (output.TryGetValue(AppArgsFlags.SKIP_AUTH_SCREEN, out string? value) && value?.EndsWith('/') == true) - output[AppArgsFlags.SKIP_AUTH_SCREEN] = value[..^1]; + // Tier 2 (SEC-019/020): the local-development params Creator Hub / sdk-commands attach to preview deep + // links (local-scene, dclenv, hub, skip-auth-screen, landscape-terrain-enabled, multi-instance) are + // permitted only when the target realm is loopback β€” a remote-realm deep link from a web page cannot + // enable them. Everything not in either tier is dropped. + bool realmIsLoopback = output.TryGetValue(AppArgsFlags.REALM, out string? loopbackRealm) + && Uri.TryCreate(loopbackRealm, UriKind.Absolute, out Uri? loopbackRealmUri) + && loopbackRealmUri.IsLoopback; + + foreach (string uriQueryKey in uriQuery.AllKeys) + { + if (uriQueryKey == null || output.ContainsKey(uriQueryKey)) continue; + + if (realmIsLoopback && DeepLinkAllowlist.IsPermittedForLoopbackRealm(uriQueryKey)) + output[uriQueryKey] = uriQuery.Get(uriQueryKey); + else + droppedKeys.Add(uriQueryKey); + } + + if (droppedKeys.Count > 0) + ReportHub.LogWarning(ReportCategory.ALWAYS, $"Dropped {droppedKeys.Count} non-allowlisted deep-link param(s): {string.Join(", ", droppedKeys)}"); return output; } diff --git a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/DeepLinkAllowlist.cs b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/DeepLinkAllowlist.cs new file mode 100644 index 00000000000..e18a351202f --- /dev/null +++ b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/DeepLinkAllowlist.cs @@ -0,0 +1,99 @@ +using System.Collections.Generic; + +namespace Global.AppArgs +{ + /// + /// Deny-by-default allowlist of query params a decentraland:// deep link may inject into app-args. + /// Shared by the cold-start argv path and the runtime bridge path (both funnel through + /// ). + /// + /// A deep link is fully attacker-controllable β€” anyone can craft one and get a victim to open it β€” so + /// params fall into three tiers: + /// + /// + /// + /// Always permitted β€” benign navigation / share / login intents whose worst case is already + /// gated elsewhere (a consent prompt, a matching login token, or a plain coordinate): realm, + /// position, community, signin, authRequestId, force-open-backpack. + /// + /// + /// Permitted only for a loopback realm β€” the local-development params Creator Hub and the + /// SDK (sdk-commands) attach to their preview deep links: local-scene, dclenv, hub, + /// skip-auth-screen, landscape-terrain-enabled, multi-instance. They are gated on + /// Uri.IsLoopback of the target realm (127.0.0.1 / localhost / [::1]) so a remote-realm deep + /// link from a web page can never enable them, while a legitimate local-dev launch (which always + /// targets loopback) works. Each is individually low-harm β€” an analytics tag, a cosmetic toggle, an + /// instance count, an env enum, or a screen skip that still forces auth when no valid identity is + /// cached β€” and the loopback gate confines them to the dev context. + /// + /// + /// Never permitted β€” everything else, in particular params that launch code + /// (creator-hub-bin-path, launch-cdp-monitor-on-start β€” SEC-005); point the client at + /// attacker infrastructure (comms-adapter, gatekeeper-url, friends-api-url β€” + /// SEC-052, feature-flags-url/-hostname, optimized-assets-url, + /// lsd-remote-ab-server/-world, pulse); bypass a version/specs screen + /// (skip-version-check, skip-minimum-specs-screen); or enable other dev/test modes + /// (debug, scene-console, autopilot, alttester, simulate*). + /// + /// + /// Both permitted sets are a product decision (SEC-019/020 "Design affected") β€” changing them requires sign-off. + /// + public static class DeepLinkAllowlist + { + private static readonly HashSet PERMITTED_KEYS = new() + { + // Which world/realm to enter. Attacker-controllable, but never applied silently: the switch is routed + // through the ChangeRealm consent prompt (SEC-004) and a host-suffix environment check. + AppArgsFlags.REALM, + + // Target parcel "x,y" to land on. A plain coordinate β€” no security impact. + AppArgsFlags.POSITION, + + // Community id shown as a "view this community" notification; opening the card is a user action. Benign UI. + AppArgsFlags.COMMUNITY, + + // Login flow: opaque identity id from the auth website's signin link. Consumed only while a local login + // is actively awaiting one AND AUTH_REQUEST_ID matches the request that minted it (see DeepLinkHandle). + AppArgsFlags.SIGNIN, + + // Login flow: binds a signin link to the login that requested it; inert without a matching pending login. + AppArgsFlags.AUTH_REQUEST_ID, + + // Opens the user's own backpack panel on landing (shipped deep-link feature). Benign in-client navigation. + AppArgsFlags.FORCE_OPEN_BACKPACK, + }; + + // Local-development params Creator Hub / sdk-commands attach to preview deep links. Permitted ONLY when the + // target realm is loopback (see ApplicationParametersParser.ProcessDeepLinkParameters) β€” a remote-realm deep + // link can never enable them. The SEC-005 exec params (creator-hub-bin-path, launch-cdp-monitor-on-start) + // are deliberately NOT here; they stay dropped for every realm. + private static readonly HashSet LOOPBACK_REALM_PERMITTED_KEYS = new() + { + // Enables local-scene-development mode (opens an LSD websocket to the realm). Only meaningful against a + // local server; loopback-gated so an attacker can't point LSD at a remote realm (SEC-020). + AppArgsFlags.LOCAL_SCENE, + + // Target environment (org/zone/today). A DCL-owned enum, not a URL β€” cannot point at attacker infra. + AppArgsFlags.ENVIRONMENT, + + // Marks the session as launched from the Creator Hub (analytics trait only β€” no capability unlock). + AppArgsFlags.DCL_EDITOR, + + // Skips the login screen. Cannot bypass authentication: auth is still forced when no valid identity is + // cached (RealUserInAppInitializationFlow). A convenience for the local-dev loop. + AppArgsFlags.SKIP_AUTH_SCREEN, + + // Toggles landscape terrain rendering. Cosmetic. + AppArgsFlags.LANDSCAPE_TERRAIN_ENABLED, + + // Allows multiple client instances (local multi-instance dev workflow). + AppArgsFlags.MULTIPLE_RUNNING_INSTANCES, + }; + + public static bool IsPermitted(string key) => + PERMITTED_KEYS.Contains(key); + + public static bool IsPermittedForLoopbackRealm(string key) => + LOOPBACK_REALM_PERMITTED_KEYS.Contains(key); + } +} diff --git a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/DeepLinkAllowlist.cs.meta b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/DeepLinkAllowlist.cs.meta new file mode 100644 index 00000000000..1a699565aa2 --- /dev/null +++ b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/DeepLinkAllowlist.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: a578a6538c5cdc84f8229ac2ead3828b \ No newline at end of file diff --git a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/Tests/AppArgsTests.cs b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/Tests/AppArgsTests.cs index baab2acbe97..8bd937184a9 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/Tests/AppArgsTests.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/AppArgs/Tests/AppArgsTests.cs @@ -48,5 +48,89 @@ public void DebugArgContainsTest() IAppArgs args = new ApplicationParametersParser(false, "--debug"); Assert.True(args.HasDebugFlag(false), $"flags in args: {string.Join(", ", args.Flags())}"); } + + [Test] + public void DeepLinkDropsInternalFlags() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?creator-hub-bin-path=%5C%5Cattacker%5Cshare%5Cp.exe&launch-cdp-monitor-on-start&local-scene=true&comms-adapter=x&skip-auth-screen=true"); + + Assert.IsFalse(output.ContainsKey("creator-hub-bin-path"), "creator-hub-bin-path must be dropped from deep links (not an app-arg; the Creator Hub path is resolved at runtime)"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.LAUNCH_CDP_MONITOR_ON_START), "launch-cdp-monitor-on-start must be dropped"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.LOCAL_SCENE), "local-scene must be dropped"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.COMMS_ADAPTER), "comms-adapter must be dropped"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.SKIP_AUTH_SCREEN), "skip-auth-screen must be dropped"); + } + + [Test] + public void DeepLinkKeepsAllowlistedParams() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?realm=https://peer.decentraland.org&position=10,20&community=abc&signin=id1&authRequestId=req1&force-open-backpack=true"); + + Assert.AreEqual("https://peer.decentraland.org", output.GetValueOrDefault(AppArgsFlags.REALM)); + Assert.AreEqual("10,20", output.GetValueOrDefault(AppArgsFlags.POSITION)); + Assert.AreEqual("abc", output.GetValueOrDefault(AppArgsFlags.COMMUNITY)); + Assert.AreEqual("id1", output.GetValueOrDefault(AppArgsFlags.SIGNIN)); + Assert.AreEqual("req1", output.GetValueOrDefault(AppArgsFlags.AUTH_REQUEST_ID)); + Assert.IsTrue(output.ContainsKey(AppArgsFlags.FORCE_OPEN_BACKPACK), "force-open-backpack must survive (shipped feature #9398)"); + } + + [Test] + public void DeepLinkKeepsLocalSceneForLoopbackRealm() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?realm=http://127.0.0.1:8000&position=100,100&local-scene=true"); + + Assert.AreEqual("true", output.GetValueOrDefault(AppArgsFlags.LOCAL_SCENE), "local-scene must survive for a loopback (local dev) realm"); + } + + [Test] + public void DeepLinkDropsLocalSceneForRemoteRealm() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?realm=https://evil.example&local-scene=true"); + + Assert.IsFalse(output.ContainsKey(AppArgsFlags.LOCAL_SCENE), "local-scene must be dropped for a non-loopback (remote) realm (SEC-020)"); + } + + [Test] + public void DeepLinkKeepsSdkAndCreatorHubDevParamsForLoopbackRealm() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?realm=http://127.0.0.1:8000&position=10,20&local-scene=true&dclenv=zone&hub=true&skip-auth-screen=true&landscape-terrain-enabled=true&multi-instance=true"); + + Assert.AreEqual("true", output.GetValueOrDefault(AppArgsFlags.LOCAL_SCENE), "local-scene"); + Assert.AreEqual("zone", output.GetValueOrDefault(AppArgsFlags.ENVIRONMENT), "dclenv"); + Assert.AreEqual("true", output.GetValueOrDefault(AppArgsFlags.DCL_EDITOR), "hub"); + Assert.AreEqual("true", output.GetValueOrDefault(AppArgsFlags.SKIP_AUTH_SCREEN), "skip-auth-screen"); + Assert.AreEqual("true", output.GetValueOrDefault(AppArgsFlags.LANDSCAPE_TERRAIN_ENABLED), "landscape-terrain-enabled"); + Assert.AreEqual("true", output.GetValueOrDefault(AppArgsFlags.MULTIPLE_RUNNING_INSTANCES), "multi-instance"); + } + + [Test] + public void DeepLinkDropsSdkAndCreatorHubDevParamsForRemoteRealm() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?realm=https://peer.decentraland.org&local-scene=true&dclenv=zone&hub=true&skip-auth-screen=true&landscape-terrain-enabled=true&multi-instance=true"); + + Assert.IsFalse(output.ContainsKey(AppArgsFlags.LOCAL_SCENE), "local-scene must be dropped for a remote realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.ENVIRONMENT), "dclenv must be dropped for a remote realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.DCL_EDITOR), "hub must be dropped for a remote realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.SKIP_AUTH_SCREEN), "skip-auth-screen must be dropped for a remote realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.LANDSCAPE_TERRAIN_ENABLED), "landscape-terrain-enabled must be dropped for a remote realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.MULTIPLE_RUNNING_INSTANCES), "multi-instance must be dropped for a remote realm"); + } + + [Test] + public void DeepLinkDropsExecAndInfraParamsEvenForLoopbackRealm() + { + Dictionary output = ApplicationParametersParser.ProcessDeepLinkParameters( + "decentraland://?realm=http://127.0.0.1:8000&creator-hub-bin-path=x&launch-cdp-monitor-on-start=true&comms-adapter=y"); + + Assert.IsFalse(output.ContainsKey("creator-hub-bin-path"), "creator-hub-bin-path must never be permitted (SEC-005), even for a loopback realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.LAUNCH_CDP_MONITOR_ON_START), "launch-cdp-monitor-on-start must never be permitted, even for a loopback realm"); + Assert.IsFalse(output.ContainsKey(AppArgsFlags.COMMS_ADAPTER), "comms-adapter must never be permitted, even for a loopback realm"); + } } } diff --git a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/BootstrapContainer.cs b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/BootstrapContainer.cs index b830f820ad2..3172fe1a5e4 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/BootstrapContainer.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/BootstrapContainer.cs @@ -128,7 +128,7 @@ await bootstrapContainer.InitializeContainerAsync chatContainer.ChatMessagesBus.SendWithUtcNowTimestamp(ChatChannel.NEARBY_CHANNEL, $"/{ChatCommandsUtils.COMMAND_GOTO} {realmUrl}", ChatMessageOrigin.RESTRICTED_ACTION_API)), + (realmUrl, position) => + { + // With a target parcel: teleport with the typed position (works for URL realms too). + // Without one: keep the existing chat-command route so the switch surfaces in nearby chat. + if (position.HasValue) + // TODO: surface the teleport result (chat bus / notification) like the no-position path below, + // and plumb a real cancellation token instead of None (composition-root fire-and-forget for now). + chatContainer.ChatTeleporter.TeleportToRealmAsync(realmUrl, position.Value, CancellationToken.None).Forget(); + else + chatContainer.ChatMessagesBus.SendWithUtcNowTimestamp(ChatChannel.NEARBY_CHANNEL, $"/{ChatCommandsUtils.COMMAND_GOTO} {realmUrl}", ChatMessageOrigin.RESTRICTED_ACTION_API); + }), new NftPromptPlugin(assetsProvisioner, webBrowser, uiShellContainer.MvcManager, nftInfoAPIClient, staticContainer.ImageControllerProvider, uiShellContainer.Cursor), staticContainer.CharacterContainer.CreateGlobalPlugin(), staticContainer.QualityContainer.CreatePlugin(), diff --git a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmLaunchSettings.cs b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmLaunchSettings.cs index 215a6289b83..6299406d016 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmLaunchSettings.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmLaunchSettings.cs @@ -1,3 +1,4 @@ +using DCL.Browser; using DCL.CommunicationData.URLHelpers; using ECS.SceneLifeCycle.Realm; using Global.AppArgs; @@ -101,7 +102,7 @@ private void ParseRealmAppParameter(IAppArgs appParameters, string realmParamVal bool isLocalSceneDevelopment = appParameters.TryGetValue(AppArgsFlags.LOCAL_SCENE, out string localSceneParamValue) && ParseLocalSceneParameter(localSceneParamValue) - && IsRealmAValidUrl(realmParamValue); + && ExternalUrlPolicy.IsWebScheme(realmParamValue); if (isLocalSceneDevelopment) { @@ -171,10 +172,6 @@ private bool ParseLocalSceneParameter(string localSceneParameter) private bool IsRealmAWorld(string realmParam) => realmParam.IsEns(); - private bool IsRealmAValidUrl(string realmParam) => - Uri.TryCreate(realmParam, UriKind.Absolute, out Uri? uriResult) - && (uriResult.Scheme == Uri.UriSchemeHttp || uriResult.Scheme == Uri.UriSchemeHttps); - public void CheckStartParcelOverride(IAppArgs appArgs, FeatureFlagsConfiguration featureFlagsConfigurationCache) { // Priority 1: App argument position (highest - from command line/Creator Hub) diff --git a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmUrl/RealmUrls.cs b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmUrl/RealmUrls.cs index 0a0fb5a1ca7..1cac06968ac 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmUrl/RealmUrls.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/Dynamic/RealmUrl/RealmUrls.cs @@ -1,4 +1,5 @@ using Cysharp.Threading.Tasks; +using DCL.Browser; using DCL.Multiplayer.Connections.DecentralandUrls; using DCL.Utility; using ECS.SceneLifeCycle.Realm; @@ -52,7 +53,7 @@ private async UniTask CustomRealmAsync(CancellationToken ct) { string realm = realmLaunchSettings.customRealm; - if (realm.StartsWith("http://", StringComparison.Ordinal) || realm.StartsWith("https://", StringComparison.Ordinal)) + if (ExternalUrlPolicy.IsWebScheme(realm)) return realm; return await realmNames.UrlFromNameAsync(realm, ct); diff --git a/Explorer/Assets/DCL/Infrastructure/Global/Editor/DebugSettingsDrawer.cs b/Explorer/Assets/DCL/Infrastructure/Global/Editor/DebugSettingsDrawer.cs index 2890ea23ae4..f82ef383e85 100644 --- a/Explorer/Assets/DCL/Infrastructure/Global/Editor/DebugSettingsDrawer.cs +++ b/Explorer/Assets/DCL/Infrastructure/Global/Editor/DebugSettingsDrawer.cs @@ -26,9 +26,8 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten SerializedProperty appParameters = property.FindPropertyRelative("appParameters"); bool cdpEnabled = HasFlag(appParameters, AppArgsFlags.LAUNCH_CDP_MONITOR_ON_START); - string? currentCreatorHubPath = GetFlagValue(appParameters, AppArgsFlags.CREATOR_HUB_BIN_PATH); - string detectedCreatorHubPath = FindCreatorHubPath(); + string? detectedCreatorHubPath = FindCreatorHubPath(); bool creatorHubInstalled = !string.IsNullOrEmpty(detectedCreatorHubPath); // CDP DevTools Section @@ -46,7 +45,7 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten else { statusStyle.normal.textColor = new Color(0.9f, 0.5f, 0.1f); - EditorGUI.LabelField(position, "Creator Hub not found at default location", statusStyle); + EditorGUI.LabelField(position, "Creator Hub not found at default location (you'll be prompted to locate it when DevTools launches)", statusStyle); } position.y += SingleLineHeight; @@ -58,62 +57,13 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten if (EditorGUI.EndChangeCheck() && newCdpEnabled != cdpEnabled) { if (newCdpEnabled) - EnableCdpDevTools(appParameters, detectedCreatorHubPath); + EnableCdpDevTools(appParameters); else DisableCdpDevTools(appParameters); } position.y += SingleLineHeight; - // Custom Creator Hub path (only if CDP is enabled) - if (cdpEnabled) - { - EditorGUI.BeginChangeCheck(); - - Rect pathFieldRect = position; - pathFieldRect.width -= 80; - - Rect browseButtonRect = position; - browseButtonRect.x = position.xMax - 75; - browseButtonRect.width = 75; - - string displayPath = currentCreatorHubPath ?? detectedCreatorHubPath ?? "Not found"; - string newPath = EditorGUI.TextField(pathFieldRect, new GUIContent("Creator Hub Path"), displayPath); - - if (GUI.Button(browseButtonRect, "Browse")) - { - string initialDir = !string.IsNullOrEmpty(currentCreatorHubPath) ? Path.GetDirectoryName(currentCreatorHubPath) : - !string.IsNullOrEmpty(detectedCreatorHubPath) ? Path.GetDirectoryName(detectedCreatorHubPath) : Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); - -#if UNITY_EDITOR_WIN - string selectedPath = EditorUtility.OpenFilePanel("Select Creator Hub Executable", initialDir ?? "", "exe"); -#else - string selectedPath = EditorUtility.OpenFilePanel("Select Creator Hub Executable", initialDir ?? "", ""); -#endif - - if (!string.IsNullOrEmpty(selectedPath)) - newPath = selectedPath; - } - - if (EditorGUI.EndChangeCheck() && newPath != displayPath) - SetCreatorHubPath(appParameters, newPath); - - position.y += SingleLineHeight; - - // Clear custom path button if a custom path is set - if (!string.IsNullOrEmpty(currentCreatorHubPath)) - { - Rect clearButtonRect = position; - clearButtonRect.x = position.xMax - 150; - clearButtonRect.width = 150; - - if (GUI.Button(clearButtonRect, "Use Default Path")) - RemoveFlag(appParameters, AppArgsFlags.CREATOR_HUB_BIN_PATH); - - position.y += SingleLineHeight; - } - } - // Separator position.y += 5; EditorGUI.LabelField(position, "", GUI.skin.horizontalSlider); @@ -127,30 +77,13 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten public override float GetPropertyHeight(SerializedProperty property, GUIContent label) { - SerializedProperty appParameters = property.FindPropertyRelative("appParameters"); - bool cdpEnabled = HasFlag(appParameters, AppArgsFlags.LAUNCH_CDP_MONITOR_ON_START); - string? currentCreatorHubPath = GetFlagValue(appParameters, AppArgsFlags.CREATOR_HUB_BIN_PATH); - - // CDP section: header + status + toggle - int lineCount = 3; - - // Custom path field (if CDP enabled) - if (cdpEnabled) - { - lineCount += 1; - - // Clear button (if custom path is set) - if (!string.IsNullOrEmpty(currentCreatorHubPath)) - lineCount += 1; - } - - // Separator - lineCount += 1; + // CDP section (header + status + toggle) + separator. + const int LINE_COUNT = 4; // Default properties (including appParameters as raw array) float defaultPropertiesHeight = GetDefaultPropertiesHeight(property); - return (lineCount * SingleLineHeight) + 5 + defaultPropertiesHeight; + return (LINE_COUNT * SingleLineHeight) + 5 + defaultPropertiesHeight; } private static Rect DrawDefaultProperties(Rect position, SerializedProperty property) @@ -261,20 +194,7 @@ private static bool HasFlag(SerializedProperty appParameters, string flag) return false; } - private static string? GetFlagValue(SerializedProperty appParameters, string flag) - { - string formattedFlag = FormatFlag(flag); - - for (int i = 0; i < appParameters.arraySize; i++) - { - if (appParameters.GetArrayElementAtIndex(i).stringValue == formattedFlag && i + 1 < appParameters.arraySize) - return appParameters.GetArrayElementAtIndex(i + 1).stringValue; - } - - return null; - } - - private static void EnableCdpDevTools(SerializedProperty appParameters, string? creatorHubPath) + private static void EnableCdpDevTools(SerializedProperty appParameters) { if (!HasFlag(appParameters, AppArgsFlags.LAUNCH_CDP_MONITOR_ON_START)) { @@ -289,33 +209,12 @@ private static void EnableCdpDevTools(SerializedProperty appParameters, string? appParameters.GetArrayElementAtIndex(insertIndex + 1).stringValue = "true"; } - // Add Creator Hub path if found - if (!string.IsNullOrEmpty(creatorHubPath)) - SetCreatorHubPath(appParameters, creatorHubPath); - appParameters.serializedObject.ApplyModifiedProperties(); } private static void DisableCdpDevTools(SerializedProperty appParameters) { RemoveFlag(appParameters, AppArgsFlags.LAUNCH_CDP_MONITOR_ON_START); - RemoveFlag(appParameters, AppArgsFlags.CREATOR_HUB_BIN_PATH); - appParameters.serializedObject.ApplyModifiedProperties(); - } - - private static void SetCreatorHubPath(SerializedProperty appParameters, string path) - { - // First remove existing path if any - RemoveFlagWithValue(appParameters, AppArgsFlags.CREATOR_HUB_BIN_PATH); - - // Add --flag and value - int insertIndex = appParameters.arraySize; - appParameters.InsertArrayElementAtIndex(insertIndex); - appParameters.GetArrayElementAtIndex(insertIndex).stringValue = FormatFlag(AppArgsFlags.CREATOR_HUB_BIN_PATH); - - appParameters.InsertArrayElementAtIndex(insertIndex + 1); - appParameters.GetArrayElementAtIndex(insertIndex + 1).stringValue = path; - appParameters.serializedObject.ApplyModifiedProperties(); } @@ -344,31 +243,5 @@ private static void RemoveFlag(SerializedProperty appParameters, string flag) } } } - - private static void RemoveFlagWithValue(SerializedProperty appParameters, string flag) - { - string formattedFlag = FormatFlag(flag); - - for (int i = appParameters.arraySize - 1; i >= 0; i--) - { - if (appParameters.GetArrayElementAtIndex(i).stringValue == formattedFlag) - { - // Remove the value first (if exists) - if (i + 1 < appParameters.arraySize) - { - string nextValue = appParameters.GetArrayElementAtIndex(i + 1).stringValue; - - // Only remove if it's not another flag - if (!nextValue.StartsWith("--")) - appParameters.DeleteArrayElementAtIndex(i + 1); - } - - // Then remove the flag - appParameters.DeleteArrayElementAtIndex(i); - appParameters.serializedObject.ApplyModifiedProperties(); - return; - } - } - } } } diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/ExternalUrlPolicy.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/ExternalUrlPolicy.cs new file mode 100644 index 00000000000..b757fe95cca --- /dev/null +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/ExternalUrlPolicy.cs @@ -0,0 +1,35 @@ +using System; + +namespace DCL.Browser +{ + /// + /// Central policy for opening external URLs: only http/https reach the OS handler. Custom schemes + /// (decentraland://, smb://, file://, mailto:, app-protocol handlers) are refused so a scene/profile + /// link can never re-invoke the launcher (SEC-028) or leak NTLM/local files (SEC-008). + /// + public static class ExternalUrlPolicy + { + /// + /// True if the URL is an absolute http/https web URL. Validates the scheme by prefix and does NOT + /// allocate a (callers that already hold a parsed Uri use it directly instead). + /// This is the single http/https predicate shared across the client β€” external-URL sink, realm + /// resolution, and realm launch settings all route through it. + /// + public static bool IsWebScheme(string? url) => + !string.IsNullOrEmpty(url) + && (url.StartsWith("https://", StringComparison.OrdinalIgnoreCase) + || url.StartsWith("http://", StringComparison.OrdinalIgnoreCase)); + + /// Trust-cache key on (scheme, host). Returns false for authority-less URIs (empty host), + /// which must never be cached (SEC-008 empty-host bypass). + public static bool TryGetTrustKey(Uri uri, out string key) + { + key = string.Empty; + if (string.IsNullOrEmpty(uri.Host)) + return false; + + key = $"{uri.Scheme}|{uri.Host}"; + return true; + } + } +} diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/ExternalUrlPolicy.cs.meta b/Explorer/Assets/DCL/NetworkDefinitions/Browser/ExternalUrlPolicy.cs.meta new file mode 100644 index 00000000000..c456ccdc392 --- /dev/null +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/ExternalUrlPolicy.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 257d886dfa7b423297aeb6210b9ba95e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/ExternalUrlPolicyShould.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/ExternalUrlPolicyShould.cs new file mode 100644 index 00000000000..204696631e2 --- /dev/null +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/ExternalUrlPolicyShould.cs @@ -0,0 +1,33 @@ +using NUnit.Framework; +using System; + +namespace DCL.Browser +{ + public class ExternalUrlPolicyShould + { + [TestCase("https://decentraland.org", true)] + [TestCase("http://example.com", true)] + [TestCase("smb://attacker/share", false)] + [TestCase("file:///etc/passwd", false)] + [TestCase("decentraland://?creator-hub-bin-path=x", false)] + [TestCase("mailto:a@b.com", false)] + [TestCase("steam://run/1", false)] + [TestCase("not a url", false)] + public void ClassifyScheme(string url, bool expected) => + Assert.AreEqual(expected, ExternalUrlPolicy.IsWebScheme(url)); + + [Test] + public void RefuseEmptyHostTrustKey() + { + var fileUri = new Uri("file:///x"); + Assert.IsFalse(ExternalUrlPolicy.TryGetTrustKey(fileUri, out _), "empty-host URIs must never be cacheable"); + } + + [Test] + public void TrustKeyIsSchemeAndHost() + { + Assert.IsTrue(ExternalUrlPolicy.TryGetTrustKey(new Uri("https://decentraland.org/x"), out string key)); + Assert.AreEqual("https|decentraland.org", key); + } + } +} diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/ExternalUrlPolicyShould.cs.meta b/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/ExternalUrlPolicyShould.cs.meta new file mode 100644 index 00000000000..bfdc6c0adc8 --- /dev/null +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/Tests/ExternalUrlPolicyShould.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 010396ed4c3e4c40bcf607597fafae41 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs b/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs index 73266de52bf..a0924961ef3 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs +++ b/Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs @@ -1,3 +1,4 @@ +using DCL.Diagnostics; using DCL.Multiplayer.Connections.DecentralandUrls; using System; using UnityEngine; @@ -16,6 +17,12 @@ public UnityAppWebBrowser(IDecentralandUrlsSource decentralandUrlsSource) public virtual void OpenUrlMainThreadOnly(string url) { + if (!ExternalUrlPolicy.IsWebScheme(url)) + { + ReportHub.LogWarning(ReportCategory.UI, "Refused to open non-web URL scheme"); + return; + } + Application.OpenURL(Uri.EscapeUriString(url)); } diff --git a/Explorer/Assets/DCL/NetworkDefinitions/DCL.Network.asmdef b/Explorer/Assets/DCL/NetworkDefinitions/DCL.Network.asmdef index ab112aff717..116eac78eb8 100644 --- a/Explorer/Assets/DCL/NetworkDefinitions/DCL.Network.asmdef +++ b/Explorer/Assets/DCL/NetworkDefinitions/DCL.Network.asmdef @@ -12,7 +12,8 @@ "GUID:1087662aaf1c5462baa91fb9484296fd", "GUID:e0cd26848372d4e5c891c569017e11f1", "GUID:13c468c9ecfc44f249fb8fb69d3365ef", - "GUID:d8b63aba1907145bea998dd612889d6b" + "GUID:d8b63aba1907145bea998dd612889d6b", + "GUID:3acaaf1301d842f498099c27ba260121" ], "includePlatforms": [], "excludePlatforms": [], diff --git a/Explorer/Assets/DCL/PluginSystem/Global/ChangeRealmPromptPlugin.cs b/Explorer/Assets/DCL/PluginSystem/Global/ChangeRealmPromptPlugin.cs index 032fd48568d..1c49af48ed6 100644 --- a/Explorer/Assets/DCL/PluginSystem/Global/ChangeRealmPromptPlugin.cs +++ b/Explorer/Assets/DCL/PluginSystem/Global/ChangeRealmPromptPlugin.cs @@ -16,14 +16,14 @@ public class ChangeRealmPromptPlugin : IDCLGlobalPlugin changeRealmCallback; + private readonly Action changeRealmCallback; private ChangeRealmPromptController? changeRealmPromptController; public ChangeRealmPromptPlugin( IAssetsProvisioner assetsProvisioner, IMVCManager mvcManager, ICursor cursor, - Action changeRealmCallback) + Action changeRealmCallback) { this.assetsProvisioner = assetsProvisioner; this.mvcManager = mvcManager; @@ -55,7 +55,7 @@ public class ChangeRealmPromptSettings : IDCLPluginSettings [field: Header(nameof(ChangeRealmPromptPlugin) + "." + nameof(ChangeRealmPromptSettings))] [field: Space] [field: SerializeField] - public AssetReferenceGameObject ChangeRealmPromptPrefab; + public AssetReferenceGameObject ChangeRealmPromptPrefab = null!; } } } diff --git a/Explorer/Assets/DCL/Prefs/DCLPrefKeys.cs b/Explorer/Assets/DCL/Prefs/DCLPrefKeys.cs index 2e931cba04a..04bb1836b51 100644 --- a/Explorer/Assets/DCL/Prefs/DCLPrefKeys.cs +++ b/Explorer/Assets/DCL/Prefs/DCLPrefKeys.cs @@ -6,6 +6,10 @@ public static class DCLPrefKeys public const string LAUNCH_COUNT = "LaunchCount"; + // Developer-selected path to the Creator Hub executable, remembered across launches so the Chrome + // DevTools bridge can relaunch it without re-prompting. Not an app-arg / deep-link input (SEC-005). + public const string CREATOR_HUB_BIN_PATH = "CreatorHub.BinPath"; + public const string PREVIOUS_SEARCHES = "previous_searches"; public const string WEB3_IDENTITY = "Web3Authentication.Identity"; diff --git a/Explorer/Assets/DCL/RuntimeDeepLink/DeepLinkHandleImplementation.cs b/Explorer/Assets/DCL/RuntimeDeepLink/DeepLinkHandleImplementation.cs index 5cd3ba6ccf7..d917115a9e5 100644 --- a/Explorer/Assets/DCL/RuntimeDeepLink/DeepLinkHandleImplementation.cs +++ b/Explorer/Assets/DCL/RuntimeDeepLink/DeepLinkHandleImplementation.cs @@ -1,5 +1,6 @@ using CommunicationData.URLHelpers; using Cysharp.Threading.Tasks; +using DCL.ChangeRealmPrompt; using DCL.Chat.Commands; using DCL.Communities; using DCL.ExplorePanel; @@ -71,11 +72,7 @@ public DeepLinkHandleResult HandleDeepLink(DeepLink deeplink) if (realm.HasValue) { - if(position.HasValue) - chatTeleporter.TeleportToRealmAsync(realm.Value.Value, position.Value, token).Forget(); - else - chatTeleporter.TeleportToRealmAsync(realm.Value.Value, token).Forget(); - + ShowRealmChangePromptAsync(realm.Value.Value, position).Forget(); handled = true; } else if (position.HasValue) @@ -105,6 +102,19 @@ public DeepLinkHandleResult HandleDeepLink(DeepLink deeplink) return handled ? DeepLinkHandleResult.CONSUMED : DeepLinkHandleResult.NO_MATCHES; } + private async UniTaskVoid ShowRealmChangePromptAsync(string realm, Vector2Int? position) + { + // Empty message β†’ the prompt renders the default confirmation text and requires an explicit click (SEC-003). + if (token.IsCancellationRequested) + return; + + await UniTask.SwitchToMainThread(token); + + var parameters = new ChangeRealmPromptController.Params(string.Empty, realm, position); + + await mvcManager.ShowAsync(ChangeRealmPromptController.IssueCommand(parameters), token); + } + private static URLDomain? RealmFrom(DeepLink deepLink) { string? rawRealm = deepLink.ValueOf(AppArgsFlags.REALM); diff --git a/Explorer/Assets/DCL/Tests/Editor/ChangeRealmPromptDestinationHostShould.cs b/Explorer/Assets/DCL/Tests/Editor/ChangeRealmPromptDestinationHostShould.cs new file mode 100644 index 00000000000..ae3abc700a0 --- /dev/null +++ b/Explorer/Assets/DCL/Tests/Editor/ChangeRealmPromptDestinationHostShould.cs @@ -0,0 +1,20 @@ +using DCL.ChangeRealmPrompt; +using NUnit.Framework; + +namespace DCL.Tests.Editor +{ + public class ChangeRealmPromptDestinationHostShould + { + [TestCase("https://evil.com/path", "evil.com")] + [TestCase("https://evil.com:8080/path", "evil.com:8080")] // port kept in the displayed authority + [TestCase("https://decentraland.org@evil.com", "evil.com")] // userinfo stripped β€” real host shown + [TestCase("https://user:pass@evil.com:443/x", "evil.com:443")] // userinfo + port + [TestCase("world-name", "world-name")] // no scheme β†’ shown unchanged + [TestCase("https://host?q=1", "host")] // query stripped + [TestCase("https://host#frag", "host")] // fragment stripped + public void ExtractsTrueHostForDisplay(string realm, string expected) + { + Assert.AreEqual(expected, ChangeRealmPromptController.DestinationHostFor(realm), realm); + } + } +} diff --git a/Explorer/Assets/DCL/Tests/Editor/ChangeRealmPromptDestinationHostShould.cs.meta b/Explorer/Assets/DCL/Tests/Editor/ChangeRealmPromptDestinationHostShould.cs.meta new file mode 100644 index 00000000000..36d472b31bb --- /dev/null +++ b/Explorer/Assets/DCL/Tests/Editor/ChangeRealmPromptDestinationHostShould.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 6937ae263a04c1b4486661b8361d27bf \ No newline at end of file diff --git a/Explorer/Assets/DCL/Tests/Editor/ChatEnvironmentValidatorShould.cs b/Explorer/Assets/DCL/Tests/Editor/ChatEnvironmentValidatorShould.cs new file mode 100644 index 00000000000..27a72d69c9a --- /dev/null +++ b/Explorer/Assets/DCL/Tests/Editor/ChatEnvironmentValidatorShould.cs @@ -0,0 +1,31 @@ +using DCL.Chat.Commands; +using DCL.Multiplayer.Connections.DecentralandUrls; +using NUnit.Framework; + +namespace DCL.Tests.Editor +{ + public class ChatEnvironmentValidatorShould + { + [TestCase("https://peer.decentraland.org", true)] + [TestCase("https://worlds-content-server.decentraland.org/world/x.dcl.eth", true)] + [TestCase("https://evil.example/org", false)] // substring "org" no longer passes + [TestCase("https://decentraland.org.attacker.com", false)] // suffix-spoof + [TestCase("https://attacker-decentraland.org", false)] // not a real subdomain boundary + [TestCase("https://decentraland.org@evil.com", false)] // userinfo spoof β€” real host is evil.com + [TestCase("https://decentraland.org:1234@evil.com", false)] // colon-before-@ userinfo spoof (real host evil.com) + [TestCase("https://peer.decentraland.org:443", true)] // legit host with an explicit port + public void ValidateOrgBySuffix(string realm, bool expectSuccess) + { + var validator = new ChatEnvironmentValidator(DecentralandEnvironment.Org); + Assert.AreEqual(expectSuccess, validator.ValidateTeleport(realm).Success, realm); + } + + [TestCase("https://peer.decentraland.zone", true)] + [TestCase("https://evil.example/zone", false)] + public void ValidateZoneBySuffix(string realm, bool expectSuccess) + { + var validator = new ChatEnvironmentValidator(DecentralandEnvironment.Zone); + Assert.AreEqual(expectSuccess, validator.ValidateTeleport(realm).Success, realm); + } + } +} diff --git a/Explorer/Assets/DCL/Tests/Editor/ChatEnvironmentValidatorShould.cs.meta b/Explorer/Assets/DCL/Tests/Editor/ChatEnvironmentValidatorShould.cs.meta new file mode 100644 index 00000000000..73ca7156280 --- /dev/null +++ b/Explorer/Assets/DCL/Tests/Editor/ChatEnvironmentValidatorShould.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 20aae7d5f1561264c8c0f06825f4a2a9 \ No newline at end of file diff --git a/Explorer/Assets/DCL/UI/AssemblyInfo.cs b/Explorer/Assets/DCL/UI/AssemblyInfo.cs new file mode 100644 index 00000000000..7debebc1e54 --- /dev/null +++ b/Explorer/Assets/DCL/UI/AssemblyInfo.cs @@ -0,0 +1,5 @@ +using System.Runtime.CompilerServices; + +// Exposes internal members (e.g. ChangeRealmPromptController.DestinationHostFor) to the EditMode test assembly +// so security-critical host parsing can be unit-tested without widening the members to public. +[assembly: InternalsVisibleTo("DCL.EditMode.Tests")] diff --git a/Explorer/Assets/DCL/UI/AssemblyInfo.cs.meta b/Explorer/Assets/DCL/UI/AssemblyInfo.cs.meta new file mode 100644 index 00000000000..fb89c5edd82 --- /dev/null +++ b/Explorer/Assets/DCL/UI/AssemblyInfo.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: d8c10cbfea997674e938377903c2ccf9 \ No newline at end of file diff --git a/Explorer/Assets/DCL/WebRequests/ChromeDevtool/ChromeDevToolHandler.cs b/Explorer/Assets/DCL/WebRequests/ChromeDevtool/ChromeDevToolHandler.cs index c6e8e7ea7d0..f62645fbbe5 100644 --- a/Explorer/Assets/DCL/WebRequests/ChromeDevtool/ChromeDevToolHandler.cs +++ b/Explorer/Assets/DCL/WebRequests/ChromeDevtool/ChromeDevToolHandler.cs @@ -3,7 +3,6 @@ using DCL.Diagnostics; using DCL.Optimization.Pools; using DCL.WebRequests.Analytics; -using Global.AppArgs; using System; using System.Collections.Generic; using System.Linq; @@ -50,10 +49,10 @@ public static ChromeDevToolHandler NewForTest() => new (1, new Bridge()); #endif - public static ChromeDevToolHandler New(bool startOnCreation, IAppArgs appArgs) + public static ChromeDevToolHandler New(bool startOnCreation) { #if UNITY_WEBGL // ChromeDevtoolProtocolClient is not supported on WebGL. WebSocket server cannot be instantiated from a webpage, use mock - return new ChromeDevToolHandler(0, null!); + return new ChromeDevToolHandler(0, null!); #else const int PORT = 1473; @@ -61,7 +60,7 @@ public static ChromeDevToolHandler New(bool startOnCreation, IAppArgs appArgs) var bridge = new Bridge( handleMethod: HandleCdpMethod, - browser: new CreatorHubBrowser(appArgs, PORT), + browser: new CreatorHubBrowser(PORT), logger: new DCLLogger(ReportCategory.CHROME_DEVTOOL_PROTOCOL), port: PORT ); diff --git a/Explorer/Assets/DCL/WebRequests/ChromeDevtool/CreatorHubBrowser.cs b/Explorer/Assets/DCL/WebRequests/ChromeDevtool/CreatorHubBrowser.cs index e50cfc4ce2f..83bb6cf81bb 100644 --- a/Explorer/Assets/DCL/WebRequests/ChromeDevtool/CreatorHubBrowser.cs +++ b/Explorer/Assets/DCL/WebRequests/ChromeDevtool/CreatorHubBrowser.cs @@ -1,6 +1,7 @@ using CDPBridges; +using Crosstales.FB; using DCL.Diagnostics; -using Global.AppArgs; +using DCL.Prefs; using Plugins.DclNativeProcesses; using RichTypes; using System; @@ -11,7 +12,7 @@ namespace DCL.WebRequests.ChromeDevtool public class CreatorHubBrowser : IBrowser { private const string DEVTOOL_PORT_ARG = "--open-devtools-with-port="; - private readonly IAppArgs appArgs; + private readonly int port; #if UNITY_EDITOR_WIN || UNITY_STANDALONE_WIN || PLATFORM_STANDALONE_WIN @@ -21,35 +22,34 @@ public class CreatorHubBrowser : IBrowser Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "Programs", "creator-hub", "Decentraland Creator Hub.exe" ); + + private const string EXECUTABLE_EXTENSION = "exe"; #else public static readonly string DEFAULT_CREATOR_HUB_BIN_PATH = "/Applications/Decentraland Creator Hub.app/Contents/MacOS/Decentraland Creator Hub"; + + private const string EXECUTABLE_EXTENSION = "*"; #endif - public CreatorHubBrowser(IAppArgs appArgs, int port) + public CreatorHubBrowser(int port) { - this.appArgs = appArgs; this.port = port; } public BrowserOpenResult OpenUrl(string url) { - string path = CreatorHubExecutablePath(); + // Resolve lazily: the CDP bridge is the only consumer of the Creator Hub path, so we only + // touch PlayerPrefs / prompt the developer when DevTools actually needs to launch it. + string path = ResolveBinPath(); - if (File.Exists(path) == false) + if (string.IsNullOrEmpty(path) || File.Exists(path) == false) { - BrowserOpenError error = BrowserOpenError.FromException(new Exception($"Creator Hub is not installed in path: {path}")); + BrowserOpenError error = BrowserOpenError.FromException(new Exception($"Creator Hub executable not found or not selected (resolved path: '{path}')")); return BrowserOpenResult.FromBrowserOpenError(error); } ReportHub.LogWarning(ReportCategory.CHROME_DEVTOOL_PROTOCOL, "Url always ignored by Creator Hub Browser, port is used"); - Result result = DclProcesses.Start( - path, - new[] - { - $"{DEVTOOL_PORT_ARG}{port}", - } - ); + Result result = DclProcesses.Start(path, new[] { $"{DEVTOOL_PORT_ARG}{port}" }); if (result.Success == false) { @@ -60,13 +60,48 @@ public BrowserOpenResult OpenUrl(string url) return BrowserOpenResult.Success(); } - private string CreatorHubExecutablePath() + /// + /// Resolves the Creator Hub executable: a path the developer previously selected (remembered in + /// ) wins; otherwise the pinned default install path; otherwise the + /// developer is prompted once to locate it and the choice is remembered. There is no app-arg / + /// deep-link input (SEC-005) β€” the path is either the known install location or a local file the + /// developer explicitly picks. + /// + private static string ResolveBinPath() { - if (appArgs.TryGetValue(AppArgsFlags.CREATOR_HUB_BIN_PATH, out string? path)) - return path!; + string savedPath = DCLPlayerPrefs.GetString(DCLPrefKeys.CREATOR_HUB_BIN_PATH); + + if (!string.IsNullOrEmpty(savedPath) && File.Exists(savedPath)) + return savedPath; + + if (File.Exists(DEFAULT_CREATOR_HUB_BIN_PATH)) + return DEFAULT_CREATOR_HUB_BIN_PATH; + + string picked = PromptForExecutable(); + + if (!string.IsNullOrEmpty(picked)) + { + DCLPlayerPrefs.SetString(DCLPrefKeys.CREATOR_HUB_BIN_PATH, picked, save: true); + return picked; + } + + return string.Empty; + } + + private static string PromptForExecutable() + { + FileBrowser fileBrowser = FileBrowser.Instance; + + // Enable synchronous picking on macOS (off by default) so the dialog fits OpenUrl's synchronous contract. + fileBrowser.AllowSyncCalls = true; + + string startDirectory = File.Exists(DEFAULT_CREATOR_HUB_BIN_PATH) + ? Path.GetDirectoryName(DEFAULT_CREATOR_HUB_BIN_PATH) ?? string.Empty + : string.Empty; + + string? selected = fileBrowser.OpenSingleFile("Select the Decentraland Creator Hub executable", startDirectory, string.Empty, EXECUTABLE_EXTENSION); - ReportHub.LogWarning(ReportCategory.CHROME_DEVTOOL_PROTOCOL, "Creator Hub path is not provided, fallback to default path"); - return DEFAULT_CREATOR_HUB_BIN_PATH; + return selected ?? string.Empty; } } } diff --git a/docs/app-arguments.md b/docs/app-arguments.md index 7f8f6938688..36a7224d2c9 100644 --- a/docs/app-arguments.md +++ b/docs/app-arguments.md @@ -337,17 +337,6 @@ decentraland://?force-open-backpack=true --- -### `creator-hub-bin-path` -**Type:** String (file path) -**Description:** Specifies a custom path to the Creator Hub binary. Used when the Creator Hub needs to be launched from a non-standard location. - -**Usage:** -```bash ---creator-hub-bin-path /path/to/creator-hub -``` - ---- - ### `use-log-matrix` **Type:** String (file path) **Description:** Enables logging to a matrix file. The value should be the path to the log matrix file.