Skip to content

Commit f2dbc2a

Browse files
tsegismontvietj
tsegismont
authored and
vietj
committed
StaticHandler should encode file names in directory listing
If the file name is not properly encoded, malicious Javascript code could be executed. Signed-off-by: Thomas Segismont <[email protected]>
1 parent 6c6aab0 commit f2dbc2a

File tree

4 files changed

+116
-60
lines changed

4 files changed

+116
-60
lines changed

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

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import io.vertx.ext.web.MIMEHeader;
2727
import io.vertx.ext.web.RoutingContext;
2828
import io.vertx.ext.web.handler.ErrorHandler;
29+
import io.vertx.ext.web.impl.Utils;
2930

3031
import java.nio.charset.StandardCharsets;
3132
import java.util.List;
@@ -151,7 +152,7 @@ private boolean sendError(RoutingContext context, String mime, int errorCode) {
151152
for (StackTraceElement elem : exception.getStackTrace()) {
152153
stack
153154
.append("<li>")
154-
.append(escapeHTML(elem.toString()))
155+
.append(Utils.escapeHTML(elem.toString()))
155156
.append("</li>");
156157
}
157158
}
@@ -200,33 +201,13 @@ private boolean sendError(RoutingContext context, String mime, int errorCode) {
200201
return false;
201202
}
202203

203-
/**
204-
* Very incomplete html escape that will escape the most common characters on error messages.
205-
* This is to avoid pulling a full dependency to perform a compliant escape. Error messages
206-
* are created by developers as such that they should not be to complex for logging.
207-
*/
208-
private static String escapeHTML(String s) {
209-
StringBuilder out = new StringBuilder(Math.max(16, s.length()));
210-
for (int i = 0; i < s.length(); i++) {
211-
char c = s.charAt(i);
212-
if (c > 127 || c == '"' || c == '\'' || c == '<' || c == '>' || c == '&') {
213-
out.append("&#");
214-
out.append((int) c);
215-
out.append(';');
216-
} else {
217-
out.append(c);
218-
}
219-
}
220-
return out.toString();
221-
}
222-
223204
private static String htmlFormat(String errorMessage) {
224205
if (errorMessage == null) {
225206
return "";
226207
}
227208

228209
// step #1 (escape html entities)
229-
String escaped = escapeHTML(errorMessage);
210+
String escaped = Utils.escapeHTML(errorMessage);
230211
// step #2 (replace line endings with breaks)
231212
return escaped.replaceAll("\\r?\\n", "<br>");
232213
}

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

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,37 +16,13 @@
1616

1717
package io.vertx.ext.web.handler.impl;
1818

19-
import static io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN;
20-
import static io.netty.handler.codec.http.HttpResponseStatus.NOT_MODIFIED;
21-
import static io.netty.handler.codec.http.HttpResponseStatus.PARTIAL_CONTENT;
22-
import static io.netty.handler.codec.http.HttpResponseStatus.REQUESTED_RANGE_NOT_SATISFIABLE;
23-
24-
import java.io.File;
25-
import java.nio.charset.Charset;
26-
import java.nio.charset.StandardCharsets;
27-
import java.util.ArrayList;
28-
import java.util.Arrays;
29-
import java.util.Collection;
30-
import java.util.Collections;
31-
import java.util.HashSet;
32-
import java.util.List;
33-
import java.util.Map;
34-
import java.util.Objects;
35-
import java.util.Set;
36-
import java.util.regex.Matcher;
37-
import java.util.regex.Pattern;
38-
3919
import io.vertx.core.AsyncResult;
4020
import io.vertx.core.Future;
4121
import io.vertx.core.Handler;
4222
import io.vertx.core.MultiMap;
4323
import io.vertx.core.file.FileProps;
4424
import io.vertx.core.file.FileSystem;
45-
import io.vertx.core.http.HttpHeaders;
46-
import io.vertx.core.http.HttpMethod;
47-
import io.vertx.core.http.HttpServerRequest;
48-
import io.vertx.core.http.HttpServerResponse;
49-
import io.vertx.core.http.HttpVersion;
25+
import io.vertx.core.http.*;
5026
import io.vertx.core.http.impl.HttpUtils;
5127
import io.vertx.core.http.impl.MimeMapping;
5228
import io.vertx.core.impl.logging.Logger;
@@ -62,6 +38,15 @@
6238
import io.vertx.ext.web.impl.ParsableMIMEValue;
6339
import io.vertx.ext.web.impl.Utils;
6440

41+
import java.io.File;
42+
import java.nio.charset.Charset;
43+
import java.nio.charset.StandardCharsets;
44+
import java.util.*;
45+
import java.util.regex.Matcher;
46+
import java.util.regex.Pattern;
47+
48+
import static io.netty.handler.codec.http.HttpResponseStatus.*;
49+
6550
/**
6651
* Static web server
6752
* Parts derived from Yoke
@@ -763,11 +748,13 @@ private void sendDirectoryListing(FileSystem fileSystem, String dir, RoutingCont
763748
}
764749
files.append("<li><a href=\"");
765750
files.append(normalizedDir);
766-
files.append(file);
751+
String encodedUriPath = Utils.encodeUriPath(file);
752+
files.append(encodedUriPath);
767753
files.append("\" title=\"");
768-
files.append(file);
754+
String escapedHTML = Utils.escapeHTML(file);
755+
files.append(escapedHTML);
769756
files.append("\">");
770-
files.append(file);
757+
files.append(escapedHTML);
771758
files.append("</a></li>");
772759
}
773760

‎vertx-web/src/main/java/io/vertx/ext/web/impl/Utils.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import io.vertx.ext.web.Route;
2525
import io.vertx.ext.web.RoutingContext;
2626

27+
import java.io.ByteArrayOutputStream;
28+
import java.nio.charset.StandardCharsets;
2729
import java.time.Instant;
2830
import java.time.LocalDateTime;
2931
import java.time.ZoneId;
@@ -234,4 +236,68 @@ public static boolean fresh(RoutingContext ctx, long lastModified) {
234236

235237
return true;
236238
}
239+
240+
/**
241+
* Very incomplete html escape that will escape the most common characters on error messages.
242+
* This is to avoid pulling a full dependency to perform a compliant escape. Error messages
243+
* are created by developers as such that they should not be to complex for logging.
244+
*/
245+
public static String escapeHTML(String s) {
246+
StringBuilder out = new StringBuilder(Math.max(16, s.length()));
247+
for (int i = 0; i < s.length(); i++) {
248+
char c = s.charAt(i);
249+
if (c > 127 || c == '"' || c == '\'' || c == '<' || c == '>' || c == '&') {
250+
out.append("&#");
251+
out.append((int) c);
252+
out.append(';');
253+
} else {
254+
out.append(c);
255+
}
256+
}
257+
return out.toString();
258+
}
259+
260+
/**
261+
* Encode the given URI path by replacing special characters as defined by RFC3986.
262+
* Adapted from Spring <a href="https://github.com/spring-projects/spring-framework/blob/16edf9867a143f95cefadb7b2115dcbbf624e176/spring-web/src/main/java/org/springframework/web/util/UriUtils.java#L179">UriUtils</a> code.
263+
*/
264+
public static String encodeUriPath(String path) {
265+
byte[] bytes = path.getBytes(StandardCharsets.UTF_8);
266+
boolean needsReplacement = false;
267+
for (byte b : bytes) {
268+
if (isAllowedForPath(b)) {
269+
needsReplacement = true;
270+
break;
271+
}
272+
}
273+
if (!needsReplacement) {
274+
return path;
275+
}
276+
return replaceCharactersForUriPath(bytes);
277+
}
278+
279+
private static String replaceCharactersForUriPath(byte[] bytes) {
280+
// Create a buffer at least twice as big as the original byte array length.
281+
// BAOS would double the buffer size the first time the min capacity is bigger than the current length anyway.
282+
ByteArrayOutputStream baos = new ByteArrayOutputStream(2 * bytes.length);
283+
for (byte b : bytes) {
284+
if (isAllowedForPath(b)) {
285+
baos.write(b);
286+
} else {
287+
baos.write('%');
288+
baos.write(Character.toUpperCase(Character.forDigit((b >> 4) & 0xF, 16)));
289+
baos.write(Character.toUpperCase(Character.forDigit(b & 0xF, 16)));
290+
}
291+
}
292+
return new String(baos.toByteArray(), StandardCharsets.UTF_8);
293+
}
294+
295+
private static boolean isAllowedForPath(byte c) {
296+
return (c >= 'a' && c <= 'z')
297+
|| (c >= 'A' && c <= 'Z')
298+
|| (c >= '0' && c <= '9')
299+
|| '-' == c || '.' == c || '_' == c || '~' == c || '!' == c || '$' == c || '&' == c || '\'' == c
300+
|| '(' == c || ')' == c || '*' == c || '+' == c || ',' == c || ';' == c || '=' == c || ':' == c
301+
|| '@' == c || '/' == c;
302+
}
237303
}

‎vertx-web/src/test/java/io/vertx/ext/web/handler/StaticHandlerTest.java‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package io.vertx.ext.web.handler;
1818

19+
import io.netty.util.internal.PlatformDependent;
1920
import io.vertx.core.*;
2021
import io.vertx.core.http.*;
2122
import io.vertx.core.json.JsonArray;
@@ -25,12 +26,14 @@
2526
import io.vertx.ext.web.Router;
2627
import io.vertx.ext.web.WebTestBase;
2728
import io.vertx.ext.web.impl.Utils;
29+
import org.junit.Assume;
2830
import org.junit.Test;
2931

3032
import java.io.File;
3133
import java.net.URL;
3234
import java.nio.charset.StandardCharsets;
3335
import java.nio.file.Files;
36+
import java.nio.file.Path;
3437
import java.util.*;
3538
import java.util.concurrent.CountDownLatch;
3639
import java.util.concurrent.atomic.AtomicBoolean;
@@ -644,7 +647,9 @@ public void testDirectoryListingJsonNoHidden() throws Exception {
644647
public void testDirectoryListingHtml() throws Exception {
645648
stat.setDirectoryListing(true);
646649

647-
testDirectoryListingHtmlCustomTemplate("META-INF/vertx/web/vertx-web-directory.html");
650+
testDirectoryListingHtmlCustomTemplate("META-INF/vertx/web/vertx-web-directory.html", "/somedir2/", "<a href=\"/\">..</a>", "<ul id=\"files\"><li><a href=\"/somedir2/foo2.json\" title=\"foo2.json\">foo2.json</a></li>" +
651+
"<li><a href=\"/somedir2/somepage.html\" title=\"somepage.html\">somepage.html</a></li>" +
652+
"<li><a href=\"/somedir2/somepage2.html\" title=\"somepage2.html\">somepage2.html</a></li></ul>");
648653
}
649654

650655
@Test
@@ -653,23 +658,40 @@ public void testCustomDirectoryListingHtml() throws Exception {
653658
String dirTemplate = "custom_dir_template.html";
654659
stat.setDirectoryTemplate(dirTemplate);
655660

656-
testDirectoryListingHtmlCustomTemplate(dirTemplate);
661+
testDirectoryListingHtmlCustomTemplate(dirTemplate, "/somedir2/", "<a href=\"/\">..</a>", "<ul id=\"files\"><li><a href=\"/somedir2/foo2.json\" title=\"foo2.json\">foo2.json</a></li>" +
662+
"<li><a href=\"/somedir2/somepage.html\" title=\"somepage.html\">somepage.html</a></li>" +
663+
"<li><a href=\"/somedir2/somepage2.html\" title=\"somepage2.html\">somepage2.html</a></li></ul>");
657664
}
658665

659-
private void testDirectoryListingHtmlCustomTemplate(String dirTemplateFile) throws Exception {
666+
@Test
667+
public void testCustomDirectoryListingHtmlEscaping() throws Exception {
668+
Assume.assumeFalse(PlatformDependent.isWindows());
669+
670+
File testDir = new File("target/test-classes/webroot/dirxss");
671+
Files.createDirectories(testDir.toPath());
672+
File dangerousFile = new File(testDir, "<img src=x onerror=alert('XSS-FILE')>.txt");
673+
Path dangerousFilePath = dangerousFile.toPath();
674+
Files.deleteIfExists(dangerousFilePath);
675+
Files.createFile(dangerousFilePath);
676+
660677
stat.setDirectoryListing(true);
661678

679+
testDirectoryListingHtmlCustomTemplate(
680+
"META-INF/vertx/web/vertx-web-directory.html",
681+
"/dirxss/",
682+
"<a href=\"/\">..</a>",
683+
"<ul id=\"files\"><li><a href=\"/dirxss/%3Cimg%20src=x%20onerror=alert('XSS-FILE')%3E.txt\" title=\"&#60;img src=x onerror=alert(&#39;XSS-FILE&#39;)&#62;.txt\">&#60;img src=x onerror=alert(&#39;XSS-FILE&#39;)&#62;.txt</a></li></ul>");
684+
}
662685

663-
String directoryTemplate = vertx.fileSystem().readFileBlocking(dirTemplateFile).toString();
686+
private void testDirectoryListingHtmlCustomTemplate(String dirTemplateFile, String path, String parentLink, String files) throws Exception {
687+
stat.setDirectoryListing(true);
664688

665-
String parentLink = "<a href=\"/\">..</a>";
666-
String files = "<ul id=\"files\"><li><a href=\"/somedir2/foo2.json\" title=\"foo2.json\">foo2.json</a></li>" +
667-
"<li><a href=\"/somedir2/somepage.html\" title=\"somepage.html\">somepage.html</a></li>" +
668-
"<li><a href=\"/somedir2/somepage2.html\" title=\"somepage2.html\">somepage2.html</a></li></ul>";
669689

670-
String expected = directoryTemplate.replace("{directory}", "/somedir2/").replace("{parent}", parentLink).replace("{files}", files);
690+
String directoryTemplate = vertx.fileSystem().readFileBlocking(dirTemplateFile).toString();
691+
692+
String expected = directoryTemplate.replace("{directory}", path).replace("{parent}", parentLink).replace("{files}", files);
671693

672-
testRequest(HttpMethod.GET, "/somedir2/", req -> req.putHeader("accept", "text/html"), resp -> resp.bodyHandler(buff -> {
694+
testRequest(HttpMethod.GET, path, req -> req.putHeader("accept", "text/html"), resp -> resp.bodyHandler(buff -> {
673695
assertEquals("text/html", resp.headers().get("content-type"));
674696
String sBuff = buff.toString();
675697
assertEquals(expected, sBuff);

0 commit comments

Comments
 (0)