diff --git a/src/main/java/org/cryptomator/launcher/VaultCreationEvent.java b/src/main/java/org/cryptomator/launcher/VaultCreationEvent.java index 107d3d0cd..bff8825f8 100644 --- a/src/main/java/org/cryptomator/launcher/VaultCreationEvent.java +++ b/src/main/java/org/cryptomator/launcher/VaultCreationEvent.java @@ -12,11 +12,11 @@ import java.util.Map; import java.util.Optional; /** - * Requests creation of a new vault from a {@code cryptomator://vault/create?name=…&template=…} deeplink. + * Requests creation of a new vault from a {@code cryptomator://vault/create#name=…&template=…} deeplink. *

* The {@code template} is a Base64URL-encoded ZIP archive holding a ready-made (signed) vault. The {@code name} fixes - * the vault's directory name and is restricted to a single, safe path segment so that resolving it against a - * user-chosen location cannot escape that location. + * the vault's directory name and is restricted to a single, safe path segment. Both parameters are carried in the URI + * fragment part. * * @param name the fixed vault name (a single, safe path segment) * @param template the Base64URL-decoded vault template @@ -29,7 +29,7 @@ public record VaultCreationEvent(String name, byte[] template) implements AppLau private static final String PATH = "/create"; /** - * Attempts to interpret the given URI as a {@code cryptomator://vault/create?name=…&template=…} deeplink. + * Attempts to interpret the given URI as a {@code cryptomator://vault/create#name=…&template=…} deeplink. * * @param uri the deeplink URI * @return the parsed event, or an empty optional if the URI's scheme, host or path do not identify a @@ -42,21 +42,21 @@ public record VaultCreationEvent(String name, byte[] template) implements AppLau if (!SCHEME.equalsIgnoreCase(uri.getScheme()) || !HOST.equalsIgnoreCase(uri.getHost()) || !PATH.equals(uri.getPath())) { return Optional.empty(); } - var params = parseQuery(uri.getRawQuery()); + var params = parseParams(uri.getRawFragment()); var name = params.get("name"); var templateParam = params.get("template"); if (name == null || name.isBlank()) { - throw new IllegalArgumentException("Missing required query parameter 'name'."); + throw new IllegalArgumentException("Missing required fragment parameter 'name'."); } if (templateParam == null || templateParam.isEmpty()) { - throw new IllegalArgumentException("Missing required query parameter 'template'."); + throw new IllegalArgumentException("Missing required fragment parameter 'template'."); } validateName(name); byte[] template; try { template = Base64.getUrlDecoder().decode(templateParam); } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Query parameter 'template' is not valid Base64URL.", e); + throw new IllegalArgumentException("Fragment parameter 'template' is not valid Base64URL.", e); } var leftoverParams = params.keySet().stream().filter(k -> !(k.equals("template") || k.equals("name"))).toList(); if (!leftoverParams.isEmpty()) { @@ -67,16 +67,16 @@ public record VaultCreationEvent(String name, byte[] template) implements AppLau private static void validateName(String name) { if (name.contains("/") || name.contains("\\") || name.contains("..") || name.equals(".")) { - throw new IllegalArgumentException("Query parameter 'name' must be a single path segment, but was '" + name + "'."); + throw new IllegalArgumentException("Fragment parameter 'name' must be a single path segment, but was '" + name + "'."); } } - private static Map parseQuery(String rawQuery) { + private static Map parseParams(String rawParams) { var params = new HashMap(); - if (rawQuery == null || rawQuery.isEmpty()) { + if (rawParams == null || rawParams.isEmpty()) { return params; } - for (var pair : rawQuery.split("&")) { + for (var pair : rawParams.split("&")) { var idx = pair.indexOf('='); if (idx < 0) { continue; diff --git a/src/test/java/org/cryptomator/launcher/VaultCreationEventTest.java b/src/test/java/org/cryptomator/launcher/VaultCreationEventTest.java index b007f54f7..db6e9e228 100644 --- a/src/test/java/org/cryptomator/launcher/VaultCreationEventTest.java +++ b/src/test/java/org/cryptomator/launcher/VaultCreationEventTest.java @@ -17,16 +17,34 @@ public class VaultCreationEventTest { @Test @DisplayName("a valid vault/create deeplink is parsed") public void testValid() { - var inTest = VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=MyVault&template=" + TEMPLATE_B64)).orElseThrow(); + var inTest = VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=MyVault&template=" + TEMPLATE_B64)).orElseThrow(); Assertions.assertEquals("MyVault", inTest.name()); Assertions.assertArrayEquals(TEMPLATE_BYTES, inTest.template()); } + @Test + @DisplayName("parameters in the query instead of the fragment are not accepted") + public void testQueryParamsRejected() { + var uri = URI.create("cryptomator://vault/create?name=MyVault&template=" + TEMPLATE_B64); + + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(uri)); + } + + @Test + @DisplayName("an encoded separator inside a value cannot forge another parameter") + public void testNoParameterInjectionViaEncodedSeparator() { + // '%26' must stay part of the name's value; decoding the fragment before splitting would turn it into a real + // separator and smuggle in a 'template' the link never carried - hence getRawFragment(), decoding per value. + var uri = URI.create("cryptomator://vault/create#name=a%26template=" + TEMPLATE_B64); + + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(uri)); + } + @Test @DisplayName("a URL-encoded name is decoded") public void testUrlEncodedName() { - var inTest = VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=My%20Vault&template=" + TEMPLATE_B64)).orElseThrow(); + var inTest = VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=My%20Vault&template=" + TEMPLATE_B64)).orElseThrow(); Assertions.assertEquals("My Vault", inTest.name()); } @@ -34,61 +52,61 @@ public class VaultCreationEventTest { @Test @DisplayName("a non-cryptomator scheme yields empty") public void testWrongScheme() { - Assertions.assertEquals(Optional.empty(), VaultCreationEvent.tryParse(URI.create("foobar://vault/create?name=MyVault&template=" + TEMPLATE_B64))); + Assertions.assertEquals(Optional.empty(), VaultCreationEvent.tryParse(URI.create("foobar://vault/create#name=MyVault&template=" + TEMPLATE_B64))); } @Test @DisplayName("an unknown host yields empty") public void testWrongHost() { - Assertions.assertEquals(Optional.empty(), VaultCreationEvent.tryParse(URI.create("cryptomator://foo/create?name=MyVault&template=" + TEMPLATE_B64))); + Assertions.assertEquals(Optional.empty(), VaultCreationEvent.tryParse(URI.create("cryptomator://foo/create#name=MyVault&template=" + TEMPLATE_B64))); } @Test @DisplayName("an unknown path yields empty") public void testWrongPath() { - Assertions.assertEquals(Optional.empty(), VaultCreationEvent.tryParse(URI.create("cryptomator://vault/bar?name=MyVault&template=" + TEMPLATE_B64))); + Assertions.assertEquals(Optional.empty(), VaultCreationEvent.tryParse(URI.create("cryptomator://vault/bar#name=MyVault&template=" + TEMPLATE_B64))); } @Test @DisplayName("a matching host is recognized case-insensitively") public void testHostCaseInsensitive() { - Assertions.assertTrue(VaultCreationEvent.tryParse(URI.create("cryptomator://VAULT/create?name=MyVault&template=" + TEMPLATE_B64)).isPresent()); + Assertions.assertTrue(VaultCreationEvent.tryParse(URI.create("cryptomator://VAULT/create#name=MyVault&template=" + TEMPLATE_B64)).isPresent()); } @Test @DisplayName("a missing name fails") public void testMissingName() { - Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?template=" + TEMPLATE_B64))); + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#template=" + TEMPLATE_B64))); } @Test @DisplayName("a blank name fails") public void testBlankName() { - Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=&template=" + TEMPLATE_B64))); + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=&template=" + TEMPLATE_B64))); } @Test @DisplayName("a missing template fails") public void testMissingTemplate() { - Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=MyVault"))); + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=MyVault"))); } @Test @DisplayName("an invalid Base64URL template fails") public void testInvalidTemplate() { - Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=MyVault&template=@@@"))); + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=MyVault&template=@@@"))); } @Test @DisplayName("a name containing a path separator fails") public void testNameWithSlash() { - Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=foo%2Fbar&template=" + TEMPLATE_B64))); + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=foo%2Fbar&template=" + TEMPLATE_B64))); } @Test @DisplayName("a name with parent-dir traversal fails") public void testNameWithTraversal() { - Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create?name=..&template=" + TEMPLATE_B64))); + Assertions.assertThrows(IllegalArgumentException.class, () -> VaultCreationEvent.tryParse(URI.create("cryptomator://vault/create#name=..&template=" + TEMPLATE_B64))); } }