Skip to content

Commit 81ac705

Browse files
committed
Regression in 4.5.5: SpringTemplateLoader rejects valid classpath templates (Spring Boot & Multi-module)
Context - The security patch in 4.5.5 (for GHSA-pvrx-3g7m-fpgv) introduced strict OS-level canonical file checks for all template prefixes. Bug - This breaks legitimate template resolution for classpath: resources in packaged Spring Boot applications (Fat JARs) and multi-module project environments. resolveBaseDirectory() attempts to map virtual or multi-root classpath locations to a single absolute OS directory, causing the startsWith() containment check to fail and throw false-positive IllegalArgumentException path traversal errors.
1 parent de148a2 commit 81ac705

2 files changed

Lines changed: 70 additions & 37 deletions

File tree

‎handlebars-springmvc/src/main/java/com/github/jknack/handlebars/springmvc/SpringTemplateLoader.java‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.springframework.context.ApplicationContext;
1818
import org.springframework.core.io.Resource;
1919
import org.springframework.core.io.ResourceLoader;
20+
import org.springframework.util.StringUtils;
2021

2122
import com.github.jknack.handlebars.io.URLTemplateLoader;
2223

@@ -58,6 +59,34 @@ public SpringTemplateLoader(final ApplicationContext applicationContext) {
5859

5960
@Override
6061
protected URL getResource(final String location) throws IOException {
62+
String prefix = getPrefix();
63+
64+
// Classpath resources: Bypass OS-level canonicalization entirely.
65+
// The ClassLoader securely handles percent-encoding (returning 404),
66+
// so logical path validation is 100% secure and naturally supports multi-root classpaths.
67+
if (prefix.startsWith(ResourceLoader.CLASSPATH_URL_PREFIX)) {
68+
String normalizedLocation = StringUtils.cleanPath(location);
69+
String normalizedPrefix = StringUtils.cleanPath(prefix);
70+
if (!normalizedPrefix.endsWith("/")) {
71+
normalizedPrefix += "/";
72+
}
73+
74+
if (!normalizedLocation.startsWith(normalizedPrefix)) {
75+
throw new IllegalArgumentException(
76+
"Path traversal attempt detected. Logical path escapes classpath prefix: " + location);
77+
}
78+
79+
Resource resource = loader.getResource(location);
80+
if (!resource.exists()) {
81+
return null;
82+
}
83+
84+
URL url = resource.getURL();
85+
validateNoUnsafeUrlComponents(url);
86+
return url;
87+
}
88+
89+
// File-system / Servlet resources: strict OS-level validation
6190
Resource resource = loader.getResource(location);
6291
if (!resource.exists()) {
6392
return null;
@@ -83,7 +112,7 @@ protected URL getResource(final String location) throws IOException {
83112
+ location);
84113
}
85114
} else {
86-
// Fallback containment for Classpath/URL resources
115+
// Fallback containment for custom URL resources (e.g., vfs:, jndi:)
87116
String decodedPath = URLDecoder.decode(url.getPath(), StandardCharsets.UTF_8);
88117

89118
// Archive URLs embed the archive location before a '!' separator
@@ -130,9 +159,6 @@ protected URL getResource(final String location) throws IOException {
130159

131160
/**
132161
* Resolves the configured prefix to a real directory so file resources can be checked against it.
133-
* A classpath: prefix resolves through the resource loader to the actual classpath directory
134-
* (e.g. target/classes/templates), unlike a literal strip which would point at the filesystem
135-
* root.
136162
*
137163
* @return The canonical base directory for the configured prefix.
138164
* @throws IOException If the prefix resource cannot be resolved.

‎handlebars-springmvc/src/test/java/com/github/jknack/handlebars/springmvc/SpringTemplateLoaderTest.java‎

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import java.io.IOException;
1313
import java.net.URL;
14+
import java.net.URLClassLoader;
1415
import java.nio.file.Files;
1516
import java.nio.file.Path;
1617

@@ -75,54 +76,31 @@ public void classpathPrefixAcceptsJarEntryUrls() throws IOException {
7576
}
7677

7778
@Test
78-
public void shouldBlockContainsBypassInClasspath() throws IOException {
79-
Resource mockResource = mock(Resource.class);
80-
when(mockResource.exists()).thenReturn(true);
81-
when(mockResource.isFile()).thenReturn(false);
82-
83-
// Simulate an attacker traversing out, but targeting a file inside another directory
84-
// that happens to include the prefix string ("/templates/") in its path.
85-
URL maliciousUrl = new URL("jar:file:/app.jar!/com/myapp/secret/templates/keys.hbs");
86-
when(mockResource.getURL()).thenReturn(maliciousUrl);
87-
88-
// Spring resolves the traversal logically before Handlebars gets it
89-
when(resourceLoader.getResource(
90-
"classpath:/templates/../../com/myapp/secret/templates/keys.hbs"))
91-
.thenReturn(mockResource);
92-
79+
public void shouldBlockContainsBypassInClasspath() {
80+
// The logical check happens before resource resolution, so we don't need mock behavior.
81+
// It catches the traversal on the logical string level immediately.
9382
IllegalArgumentException exception =
9483
assertThrows(
9584
IllegalArgumentException.class,
9685
() -> {
9786
templateLoader.sourceAt("../../com/myapp/secret/templates/keys");
9887
});
9988

100-
assertTrue(exception.getMessage().contains("escapes Spring base prefix"));
89+
assertTrue(exception.getMessage().contains("Logical path escapes classpath prefix"));
10190
}
10291

10392
@Test
104-
public void shouldEnforceLogicalBoundaryAgainstTraversal() throws IOException {
105-
Resource mockResource = mock(Resource.class);
106-
when(mockResource.exists()).thenReturn(true);
107-
when(mockResource.isFile()).thenReturn(false);
108-
109-
URL maliciousUrl = new URL("file:///application.properties.hbs");
110-
when(mockResource.getURL()).thenReturn(maliciousUrl);
111-
112-
when(resourceLoader.getResource("classpath:/templates/../../application.properties.hbs"))
113-
.thenReturn(mockResource);
114-
115-
String maliciousPath = "../../application.properties";
116-
93+
public void shouldEnforceLogicalBoundaryAgainstTraversal() {
94+
// The logical check happens before resource resolution, so we don't need mock behavior.
95+
// It catches the traversal on the logical string level immediately.
11796
IllegalArgumentException exception =
11897
assertThrows(
11998
IllegalArgumentException.class,
12099
() -> {
121-
// sourceAt triggers resolve() and then getResource()
122-
templateLoader.sourceAt(maliciousPath);
100+
templateLoader.sourceAt("../../application.properties");
123101
});
124102

125-
assertTrue(exception.getMessage().contains("escapes Spring base"));
103+
assertTrue(exception.getMessage().contains("Logical path escapes classpath prefix"));
126104
}
127105

128106
@Test
@@ -223,11 +201,40 @@ public void shouldBlockPartialDirectoryMatch() throws IOException {
223201

224202
// Use "../" to back out of the Handlebars-enforced slash, traversing into the sibling
225203
// directory.
226-
// Handlebars resolves this to: .../templates/../templates-secret/secret.hbs
227204
IllegalArgumentException exception =
228205
assertThrows(
229206
IllegalArgumentException.class, () -> loader.sourceAt("../templates-secret/secret"));
230207

231208
assertTrue(exception.getMessage().contains("escapes Spring base"));
232209
}
210+
211+
@Test
212+
public void shouldResolveTemplatesAcrossMultiModuleClasspath() throws IOException {
213+
Path root = Files.createTempDirectory("handlebars-multimodule");
214+
215+
// Simulate Module A (e.g., target/classes/)
216+
Path moduleA = Files.createDirectory(root.resolve("moduleA"));
217+
Path templatesA = Files.createDirectory(moduleA.resolve("templates"));
218+
Files.writeString(templatesA.resolve("template-a.hbs"), "Template A");
219+
220+
// Simulate Module B (e.g., target/test-classes/ or a sibling module)
221+
Path moduleB = Files.createDirectory(root.resolve("moduleB"));
222+
Path templatesB = Files.createDirectory(moduleB.resolve("templates"));
223+
Files.writeString(templatesB.resolve("template-b.hbs"), "Template B");
224+
225+
// Create a ClassLoader containing both roots
226+
URLClassLoader classLoader =
227+
new URLClassLoader(
228+
new URL[] {moduleA.toUri().toURL(), moduleB.toUri().toURL()},
229+
getClass().getClassLoader());
230+
231+
// Use Spring's DefaultResourceLoader backed by our multi-root classloader
232+
SpringTemplateLoader loader = new SpringTemplateLoader(new DefaultResourceLoader(classLoader));
233+
loader.setPrefix("classpath:/templates/");
234+
loader.setSuffix(".hbs");
235+
236+
// The loader should find templates in both roots flawlessly without canonical file mismatches
237+
assertNotNull(loader.sourceAt("template-a"));
238+
assertNotNull(loader.sourceAt("template-b"));
239+
}
233240
}

0 commit comments

Comments
 (0)