Skip to content

Commit 14ce53f

Browse files
authored
Fix encoded path traversal corruption in sub-router handlers (#2900) (#2903)
See #2898 When a request with encoded dot-segments (e.g. %2e%2e%2f) hit a handler mounted inside a sub-router via a regex or parameterized route, Utils.pathOffset corrupted the route-relative path because dot-segment resolution had already consumed the mount-point prefix. Fix by running pathOffset on the raw normalizedPath first, then decoding and resolving dot-segments on the result. Also remove the now redundant decodeURIComponent and removeDotSegments calls from StaticHandlerImpl.handle(), since normalizedPath() already decodes unreserved characters and resolves dot-segments, and file path resolution is now fully handled in getFile(). Some portions of this content were created with the assistance of Claude Code. Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
1 parent e312bfc commit 14ce53f

3 files changed

Lines changed: 85 additions & 18 deletions

File tree

vertx-web/src/main/java/io/vertx/ext/web/handler/impl/StaticHandlerImpl.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -146,16 +146,7 @@ public void handle(RoutingContext context) {
146146
if (!request.isEnded()) {
147147
request.pause();
148148
}
149-
// decode URL path
150-
String uriDecodedPath = RFC3986.decodeURIComponent(context.normalizedPath(), false);
151-
// if the normalized path is null it cannot be resolved
152-
if (uriDecodedPath == null) {
153-
LOG.warn("Invalid path: " + context.request().path());
154-
context.next();
155-
return;
156-
}
157-
// will normalize and handle all paths as UNIX paths
158-
String path = RFC3986.removeDotSegments(uriDecodedPath.replace('\\', '/'));
149+
String path = context.normalizedPath();
159150

160151
// Access fileSystem once here to be safe
161152
FileSystem fs = context.vertx().fileSystem();
@@ -178,7 +169,7 @@ private void sendStatic(RoutingContext context, FileSystem fileSystem, String pa
178169
String file = null;
179170

180171
if (!includeHidden) {
181-
file = getFile(path, context);
172+
file = getFile(context);
182173
for (int idx = file.indexOf('/'); idx >= 0; idx = file.indexOf('/', idx + 1)) {
183174
String name = file.substring(idx + 1);
184175
if (name.length() > 0 && name.charAt(0) == '.') {
@@ -226,7 +217,7 @@ private void sendStatic(RoutingContext context, FileSystem fileSystem, String pa
226217
final String localFile;
227218

228219
if (file == null) {
229-
String ctxFile = getFile(path, context);
220+
String ctxFile = getFile(context);
230221
if (index) {
231222
localFile = ctxFile + indexPage;
232223
} else {
@@ -639,8 +630,10 @@ public StaticHandler setDefaultContentEncoding(String contentEncoding) {
639630
return this;
640631
}
641632

642-
private String getFile(String path, RoutingContext context) {
643-
String file = webRoot + Utils.pathOffset(path, context);
633+
private String getFile(RoutingContext context) {
634+
String offsetPath = Utils.pathOffset(context.normalizedPath(), context);
635+
offsetPath = RFC3986.decodeURIComponent(offsetPath, false);
636+
String file = webRoot + RFC3986.removeDotSegments(offsetPath.replace('\\', '/'));
644637
if (LOG.isTraceEnabled()) {
645638
LOG.trace("File to serve is " + file);
646639
}

vertx-web/src/main/java/io/vertx/ext/web/handler/impl/TemplateHandlerImpl.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,9 @@ public TemplateHandlerImpl(TemplateEngine engine, String templateDirectory, Stri
4545

4646
@Override
4747
public void handle(RoutingContext context) {
48-
String uriDecodedPath = RFC3986.decodeURIComponent(context.normalizedPath(), false);
49-
String path = RFC3986.removeDotSegments(uriDecodedPath.replace('\\', '/'));
50-
51-
String file = Utils.pathOffset(path, context);
48+
String file = Utils.pathOffset(context.normalizedPath(), context);
49+
file = RFC3986.decodeURIComponent(file, false);
50+
file = RFC3986.removeDotSegments(file.replace('\\', '/'));
5251
if (file.endsWith("/") && null != indexTemplate) {
5352
file += indexTemplate;
5453
}

vertx-web/src/test/java/io/vertx/ext/web/tests/SubRouterTest.java

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,19 @@
1717
package io.vertx.ext.web.tests;
1818

1919
import io.netty.handler.codec.http.HttpResponseStatus;
20+
import io.vertx.core.Future;
2021
import io.vertx.core.Handler;
2122
import io.vertx.core.Vertx;
23+
import io.vertx.core.buffer.Buffer;
2224
import io.vertx.core.http.HttpMethod;
2325
import io.vertx.ext.web.Router;
2426
import io.vertx.ext.web.RoutingContext;
27+
import io.vertx.ext.web.common.template.TemplateEngine;
28+
import io.vertx.ext.web.handler.StaticHandler;
29+
import io.vertx.ext.web.handler.TemplateHandler;
2530
import org.junit.Test;
2631

32+
import java.util.Map;
2733
import java.util.function.Consumer;
2834

2935
import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR;
@@ -847,4 +853,73 @@ public void testSubRouterMethodNotAllowed() throws Exception {
847853
assertTrue(allowHeader.contains("PUT"));
848854
}, 405, "Method Not Allowed", null);
849855
}
856+
857+
@Test
858+
public void testStaticHandlerEncodedPathTraversalWithWildcardRoute() throws Exception {
859+
Router subRouter = Router.router(vertx);
860+
router.route("/sub/*").subRouter(subRouter);
861+
subRouter.route("/*").handler(StaticHandler.create());
862+
testRequest(HttpMethod.GET, "/sub/%2e%2e%2fotherpage.html", 200, "OK", "<html><body>Other page</body></html>");
863+
}
864+
865+
@Test
866+
public void testStaticHandlerEncodedPathTraversalWithRegexRoute() throws Exception {
867+
Router subRouter = Router.router(vertx);
868+
router.route("/sub/*").subRouter(subRouter);
869+
subRouter.getWithRegex(".+\\.html").handler(StaticHandler.create());
870+
testRequest(HttpMethod.GET, "/sub/%2e%2e%2fotherpage.html", 200, "OK", "<html><body>Other page</body></html>");
871+
}
872+
873+
@Test
874+
public void testStaticHandlerEncodedPathTraversalWithPathParamRoute() throws Exception {
875+
Router subRouter = Router.router(vertx);
876+
router.route("/sub/*").subRouter(subRouter);
877+
subRouter.get("/:file").handler(StaticHandler.create());
878+
testRequest(HttpMethod.GET, "/sub/%2e%2e%2fotherpage.html", 200, "OK", "<html><body>Other page</body></html>");
879+
}
880+
881+
@Test
882+
public void testTemplateHandlerEncodedPathTraversalWithWildcardRoute() throws Exception {
883+
RecordingTemplateFileNameEngine engine = new RecordingTemplateFileNameEngine();
884+
Router subRouter = Router.router(vertx);
885+
router.route("/sub/*").subRouter(subRouter);
886+
subRouter.route("/*").handler(TemplateHandler.create(engine, "templates", "text/html"));
887+
testRequest(HttpMethod.GET, "/sub/%2e%2e%2foutside.html", 200, "OK");
888+
assertEquals("templates/outside.html", engine.lastTemplateFileName);
889+
}
890+
891+
@Test
892+
public void testTemplateHandlerEncodedPathTraversalWithRegexRoute() throws Exception {
893+
RecordingTemplateFileNameEngine engine = new RecordingTemplateFileNameEngine();
894+
Router subRouter = Router.router(vertx);
895+
router.route("/sub/*").subRouter(subRouter);
896+
subRouter.getWithRegex(".+\\.html").handler(TemplateHandler.create(engine, "templates", "text/html"));
897+
testRequest(HttpMethod.GET, "/sub/%2e%2e%2foutside.html", 200, "OK");
898+
assertEquals("templates/outside.html", engine.lastTemplateFileName);
899+
}
900+
901+
@Test
902+
public void testTemplateHandlerEncodedPathTraversalWithPathParamRoute() throws Exception {
903+
RecordingTemplateFileNameEngine engine = new RecordingTemplateFileNameEngine();
904+
Router subRouter = Router.router(vertx);
905+
router.route("/sub/*").subRouter(subRouter);
906+
subRouter.get("/:page").handler(TemplateHandler.create(engine, "templates", "text/html"));
907+
testRequest(HttpMethod.GET, "/sub/%2e%2e%2foutside.html", 200, "OK");
908+
assertEquals("templates/outside.html", engine.lastTemplateFileName);
909+
}
910+
911+
private static class RecordingTemplateFileNameEngine implements TemplateEngine {
912+
913+
String lastTemplateFileName;
914+
915+
@Override
916+
public Future<Buffer> render(Map<String, Object> context, String templateFileName) {
917+
this.lastTemplateFileName = templateFileName;
918+
return Future.succeededFuture(Buffer.buffer());
919+
}
920+
921+
@Override
922+
public void clearCache() {
923+
}
924+
}
850925
}

0 commit comments

Comments
 (0)