This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 87307615bca1c82179ca3a80ef89ff5de82d8d37 Author: Emond Papegaaij <[email protected]> AuthorDate: Mon Aug 3 16:55:07 2026 +0200 Validate locale/style/variation decoded from resource URLs The locale, style and variation decoded from a resource URL each become a single component of the resource lookup path, which ResourceNameIterator builds as <path>_<variation>_<style>_<locale>.<extension>. They were used as decoded, so a value containing a path separator contributed more than one component and the lookup resolved in a different directory than the resource it belongs to. None of the three can legitimately contain a path separator, so they are now rejected where they are decoded, in ResourceUtil#decodeResourceReferenceAttributes: the locale as the raw string before parseLocale, and the style and variation after unescapeAttributesSeparator, so that a separator written as '~' is caught after it has been restored to '-'. A rejected value is dropped and logged, leaving the request to resolve the resource under its remaining attributes rather than fail. ResourceUtil#rejectPathSeparators is public and applied a second time in the ResourceNameIterator constructor, so that callers which do not come from a URL - an application deriving a style from a request parameter, say - are covered where the attributes reach the path. The locale needed covering as well as the style and variation: parseLocale passes its argument to Locale.of, which does not validate it, and LocaleResourceNameIterator then appends Locale#toString() to the path. A value placed entirely in the locale reached the path the same way, limited to lowercase because parseLocale lowercases its input. A style or variation legitimately containing '/' or '\' no longer round-trips. Such a value never resolved to the resource it named, so the change is intended. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../ResourceUrlAttributeValidationTest.java | 225 +++++++++++++++++++++ .../apache/wicket/resource/ResourceUtilTest.java | 67 ++++++ .../resource/locator/ResourceNameIterator.java | 9 +- .../org/apache/wicket/resource/ResourceUtil.java | 71 ++++++- 4 files changed, 365 insertions(+), 7 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/request/resource/ResourceUrlAttributeValidationTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/request/resource/ResourceUrlAttributeValidationTest.java new file mode 100644 index 0000000000..5543281659 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/request/resource/ResourceUrlAttributeValidationTest.java @@ -0,0 +1,225 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.request.resource; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.util.file.IResourceFinder; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.Test; + +/** + * Verifies that the locale, style and variation decoded from a resource URL cannot change the + * directory a resource is resolved in. + * <p> + * The resource stream locator appends these three attributes to the path it asks every + * {@link IResourceFinder} for, as {@code <path>_<variation>_<style>_<locale>.<extension>}. A value + * containing a path separator would therefore contribute more than one path component, and the + * lookup would resolve somewhere other than the package the resource belongs to. No legitimate + * locale, style or variation contains one, so {@code ResourceUtil#decodeResourceReferenceAttributes} + * drops such a value. + * <p> + * These tests replace the application's resource finders with a recording one, so that the paths + * Wicket asks for can be asserted directly, independently of how a servlet container resolves them. + * {@link NormalizingFinder} additionally collapses {@code ..} the way a container does when + * resolving a path. {@code MockServletContext} does not: it resolves through + * {@code new File(root, name)}, which will not walk through a {@code PublicPage_..} component that + * does not exist, so a test relying on the mock context would not exercise this at all. + */ +public class ResourceUrlAttributeValidationTest extends WicketTestCase +{ + /** Served by {@link NormalizingFinder} for a path that resolved outside the package. */ + private static final String OUTSIDE_CONTENT = "content-from-another-location"; + + /** Where the style below points, relative to the root the finder resolves against. */ + private static final String OUTSIDE_PREFIX = "other-package/"; + + private static final String OUTSIDE_NAME = OUTSIDE_PREFIX + "Config"; + + private static final String PACKAGE_PREFIX = + ResourceUrlAttributeValidationTest.class.getPackageName().replace('.', '/') + "/"; + + /** + * How many {@code ../} the style needs to reach the root the finder resolves against. The style + * is appended as {@code <package>/<name>_<style>}, so its first {@code ..} is glued onto + * {@code PublicPage_} and forms a literal path component rather than a parent reference - hence + * one extra, plus one to consume that component itself. + */ + private static final int PARENT_STEPS = + ResourceUrlAttributeValidationTest.class.getPackageName().split("\\.").length + 2; + + /** + * The style is carried in the first query parameter's NAME, with an empty value. The {@code -} is + * the attribute separator, so a {@code -} within the style itself has to be written as + * {@code ~}: {@code ResourceUtil#unescapeAttributesSeparator} turns {@code (\w)~(\w)} back into + * {@code -} after the split. Keeping one here means the validation is also shown to happen after + * that restoration rather than before it. + */ + private static final String STYLE_WITH_SEPARATORS = + "en-" + "..%2F".repeat(PARENT_STEPS) + OUTSIDE_NAME.replace("-", "~").replace("/", "%2F"); + + private static final String URL = "wicket/resource/" + + ResourceUrlAttributeValidationTest.class.getName() + "/PublicPage.html?" + + STYLE_WITH_SEPARATORS; + + /** Referenced by the resource URL above; it does not need to exist as a file. */ + public static class PublicPage extends WebPage + { + } + + /** Records every path the locator asks for. */ + private static class RecordingFinder implements IResourceFinder + { + final List<String> asked = new ArrayList<>(); + + @Override + public IResourceStream find(Class<?> clazz, String pathname) + { + asked.add(pathname); + return found(pathname); + } + + IResourceStream found(String pathname) + { + return null; + } + } + + /** + * Collapses {@code ..} in the path string the way a servlet container does when resolving it, and + * returns content for a path that ends up under {@link #OUTSIDE_PREFIX} - that is, for a lookup + * that left the package it started in. + */ + private static class NormalizingFinder extends RecordingFinder + { + @Override + IResourceStream found(String pathname) + { + return normalize(pathname).startsWith(OUTSIDE_PREFIX) + ? new StringResourceStream(OUTSIDE_CONTENT) + : null; + } + + private static String normalize(String path) + { + List<String> segments = new ArrayList<>(); + for (String segment : path.split("/", -1)) + { + if (segment.isEmpty() || ".".equals(segment)) + { + continue; + } + if ("..".equals(segment)) + { + if (segments.isEmpty() == false) + { + segments.remove(segments.size() - 1); + } + continue; + } + segments.add(segment); + } + return String.join("/", segments); + } + } + + private <T extends RecordingFinder> T install(T finder) + { + List<IResourceFinder> finders = tester.getApplication() + .getResourceSettings() + .getResourceFinders(); + finders.clear(); + finders.add(finder); + return finder; + } + + /** + * A rejected style is dropped rather than failing the request, so the resource is still located + * under its unstyled name. + */ + @Test + void unstyledResourceIsStillLocatedWhenStyleIsRejected() + { + RecordingFinder finder = install(new RecordingFinder()); + + tester.executeUrl(URL); + + assertTrue(finder.asked.contains(PACKAGE_PREFIX + "PublicPage.html"), + "the unstyled resource should still be looked up; asked: " + finder.asked); + } + + /** + * Every path the locator asks for stays within the package, for separators carried by the style. + */ + @Test + void pathFromStyleStaysWithinPackage() + { + RecordingFinder finder = install(new RecordingFinder()); + + tester.executeUrl(URL); + + assertFalse(finder.asked.stream().anyMatch(path -> path.contains("..")), + "no path should contain a parent reference; asked: " + finder.asked); + assertTrue(finder.asked.stream().allMatch(path -> path.startsWith(PACKAGE_PREFIX)), + "every path should start with " + PACKAGE_PREFIX + "; asked: " + finder.asked); + } + + /** + * As {@link #pathFromStyleStaysWithinPackage()}, for the locale: {@code ResourceUtil#parseLocale} + * hands its input to {@code Locale.of}, which does not validate it, and + * {@code LocaleResourceNameIterator} then appends {@code Locale#toString()} to the path. The + * locale may contain neither {@code -} (the attribute separator) nor {@code _} (the locale + * separator), and is lowercased by {@code parseLocale}. + */ + @Test + void pathFromLocaleStaysWithinPackage() + { + RecordingFinder finder = install(new RecordingFinder()); + + tester.executeUrl("wicket/resource/" + + ResourceUrlAttributeValidationTest.class.getName() + "/PublicPage.html?" + + "..%2F".repeat(PARENT_STEPS) + "otherpackage%2Fconfig"); + + assertFalse(finder.asked.stream().anyMatch(path -> path.contains("..")), + "no path should contain a parent reference; asked: " + finder.asked); + assertTrue(finder.asked.stream().allMatch(path -> path.startsWith(PACKAGE_PREFIX)), + "every path should start with " + PACKAGE_PREFIX + "; asked: " + finder.asked); + } + + /** + * On a finder that collapses {@code ..} the way a container does, a path built from a style + * carrying separators would resolve outside the package. Nothing found there may be returned to + * the client. + */ + @Test + void resourceResolvingOutsideThePackageIsNotServed() + { + install(new NormalizingFinder()); + + tester.executeUrl(URL); + + assertFalse(tester.getLastResponseAsString().contains(OUTSIDE_CONTENT), + "a resource resolved outside the package must not be served"); + } +} diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/resource/ResourceUtilTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/resource/ResourceUtilTest.java index 92b84bac25..071e2447b1 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/resource/ResourceUtilTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/resource/ResourceUtilTest.java @@ -61,6 +61,73 @@ class ResourceUtilTest assertNull(attributes.getVariation()); } + @Test + void rejectPathSeparators() throws Exception + { + assertEquals("style", ResourceUtil.rejectPathSeparators("style", "style")); + assertEquals("my-style", ResourceUtil.rejectPathSeparators("my-style", "style")); + assertEquals("", ResourceUtil.rejectPathSeparators("", "style")); + assertNull(ResourceUtil.rejectPathSeparators(null, "style")); + + assertNull(ResourceUtil.rejectPathSeparators("a/b", "style")); + assertNull(ResourceUtil.rejectPathSeparators("a\\b", "style")); + assertNull(ResourceUtil.rejectPathSeparators("..", "style")); + assertNull(ResourceUtil.rejectPathSeparators("../../etc", "style")); + assertNull(ResourceUtil.rejectPathSeparators("a\0b", "style")); + assertNull(ResourceUtil.rejectPathSeparators("\0", "style")); + } + + @Test + void rejectPathSeparatorsForLocale() throws Exception + { + assertEquals(Locale.UK, ResourceUtil.rejectPathSeparators(Locale.UK)); + assertNull(ResourceUtil.rejectPathSeparators((Locale)null)); + + assertNull(ResourceUtil.rejectPathSeparators(Locale.of("../../etc"))); + assertNull(ResourceUtil.rejectPathSeparators(Locale.of("a/b"))); + } + + /** + * A locale, style or variation carrying a path separator is dropped: each becomes a single + * component of the resource lookup path, so a separator would make the lookup resolve in a + * different directory than the resource it belongs to. + */ + @Test + void decodeResourceReferenceAttributesRejectsPathSeparators() throws Exception + { + for (String value : new String[] { "../../etc", "..\\..\\etc", "a/b", "a\\b", "..", + "a\0b" }) + { + UrlAttributes attributes = ResourceUtil.decodeResourceReferenceAttributes(value); + assertNull(attributes.getLocale(), "locale should be dropped for '" + value + "'"); + + attributes = ResourceUtil.decodeResourceReferenceAttributes("en-" + value); + assertEquals(Locale.ENGLISH, attributes.getLocale()); + assertNull(attributes.getStyle(), "style should be dropped for '" + value + "'"); + + attributes = ResourceUtil.decodeResourceReferenceAttributes("en-style-" + value); + assertEquals(Locale.ENGLISH, attributes.getLocale()); + assertEquals("style", attributes.getStyle()); + assertNull(attributes.getVariation(), + "variation should be dropped for '" + value + "'"); + } + } + + /** + * The separator check runs after {@code ~} has been restored to {@code -}, and must not disturb + * that restoration. + */ + @Test + void decodeResourceReferenceAttributesKeepsEscapedSeparator() throws Exception + { + UrlAttributes attributes = + ResourceUtil.decodeResourceReferenceAttributes("en-my~style-my~variation"); + + assertEquals(Locale.ENGLISH, attributes.getLocale()); + assertEquals("my-style", attributes.getStyle()); + assertEquals("my-variation", attributes.getVariation()); + } + @Test void decodeResourceReferenceAttributesWithUrl() throws Exception { diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java index a659047d61..f89c959a3a 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Locale; import org.apache.wicket.WicketRuntimeException; +import org.apache.wicket.resource.ResourceUtil; import org.apache.wicket.util.string.Strings; /** @@ -90,7 +91,9 @@ public class ResourceNameIterator implements IResourceNameIterator public ResourceNameIterator(final String path, final String style, final String variation, final Locale locale, final Iterable<String> extensions, final boolean strict) { - this.locale = locale; + // the style, variation and locale each become a single component of the paths built below, so + // a value carrying a path separator would resolve in a different directory than the resource + this.locale = ResourceUtil.rejectPathSeparators(locale); boolean noext = extensions == null || !extensions.iterator().hasNext(); @@ -106,7 +109,9 @@ public class ResourceNameIterator implements IResourceNameIterator this.path = path; } - styleIterator = newStyleAndVariationResourceNameIterator(style, variation); + styleIterator = newStyleAndVariationResourceNameIterator( + ResourceUtil.rejectPathSeparators(style, "style"), + ResourceUtil.rejectPathSeparators(variation, "variation")); this.strict = strict; } diff --git a/wicket-core/src/main/java/org/apache/wicket/resource/ResourceUtil.java b/wicket-core/src/main/java/org/apache/wicket/resource/ResourceUtil.java index 2ef46d1957..82402843e9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/resource/ResourceUtil.java +++ b/wicket-core/src/main/java/org/apache/wicket/resource/ResourceUtil.java @@ -30,17 +30,74 @@ import org.apache.wicket.util.lang.Args; import org.apache.wicket.util.resource.IResourceStream; import org.apache.wicket.util.resource.ResourceStreamNotFoundException; import org.apache.wicket.util.string.Strings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Utilities for resources. - * + * * @author Jeremy Thomerson */ public class ResourceUtil { + private static final Logger log = LoggerFactory.getLogger(ResourceUtil.class); private static final Pattern ESCAPED_ATTRIBUTE_PATTERN = Pattern.compile("(\\w)~(\\w)"); + /** + * Rejects a resource attribute that cannot be used as a single path component. + * <p> + * The locale, style and variation are appended to the resource lookup path by + * {@link org.apache.wicket.core.util.resource.locator.ResourceNameIterator}, as + * {@code <path>_<variation>_<style>_<locale>}. A value containing a path separator would + * therefore contribute more than one component and the lookup would resolve in a different + * directory than the resource it belongs to. None of the three can legitimately contain one. + * + * @param attribute + * the attribute, may be {@code null} + * @param attributeName + * the attribute's name, used for logging + * @return the attribute, or {@code null} if it contains {@code /}, {@code \}, {@code ..} or a + * NUL character + */ + public static String rejectPathSeparators(final String attribute, final String attributeName) + { + if (attribute == null) + { + return null; + } + + if (attribute.contains("/") || attribute.contains("\\") || attribute.contains("..") || + attribute.contains("\0")) + { + log.warn("Ignoring the {} because it contains a path separator or NUL: {}", + attributeName, attribute); + + return null; + } + + return attribute; + } + + /** + * Rejects a locale whose {@link Locale#toString()} cannot be used as a single path component. + * + * @param locale + * the locale, may be {@code null} + * @return the locale, or {@code null} if its string representation contains a path separator + * + * @see #rejectPathSeparators(String, String) + */ + public static Locale rejectPathSeparators(final Locale locale) + { + if (locale == null || rejectPathSeparators(locale.toString(), "locale") != null) + { + return locale; + } + + return null; + } + /** * Reads resource reference attributes (style, locale, variation) encoded in the given string. * @@ -59,15 +116,19 @@ public class ResourceUtil if (Strings.isEmpty(encodedAttributes) == false) { String split[] = Strings.split(encodedAttributes, '-'); - locale = parseLocale(split[0]); + locale = parseLocale(rejectPathSeparators(split[0], "locale")); if (split.length == 2) { - style = Strings.defaultIfEmpty(unescapeAttributesSeparator(split[1]), null); + style = rejectPathSeparators( + Strings.defaultIfEmpty(unescapeAttributesSeparator(split[1]), null), "style"); } else if (split.length == 3) { - style = Strings.defaultIfEmpty(unescapeAttributesSeparator(split[1]), null); - variation = Strings.defaultIfEmpty(unescapeAttributesSeparator(split[2]), null); + style = rejectPathSeparators( + Strings.defaultIfEmpty(unescapeAttributesSeparator(split[1]), null), "style"); + variation = rejectPathSeparators( + Strings.defaultIfEmpty(unescapeAttributesSeparator(split[2]), null), + "variation"); } } return new ResourceReference.UrlAttributes(locale, style, variation);
