Skip to content

Commit 98b5a78

Browse files
committed
rework URI path sanitization to avoid Path
1 parent 448e25d commit 98b5a78

File tree

3 files changed

+23
-21
lines changed

3 files changed

+23
-21
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/web/Laundromat.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.jetbrains.annotations.NotNull;
2626
import org.jetbrains.annotations.Nullable;
2727

28-
import java.nio.file.Path;
2928
import java.util.ArrayList;
3029
import java.util.Arrays;
3130
import java.util.Collection;
@@ -40,7 +39,7 @@
4039
*/
4140
public class Laundromat {
4241

43-
private static final String ESC_N_R_T_F = "[\\n\\r\\t\\f]";
42+
private static final String ESC_N_R_T_F = "[\\n\\r\\t\\f\\u0000]";
4443
private static final String ESG_N_R_T_F_1_N = ESC_N_R_T_F + "+";
4544

4645
/**
@@ -75,21 +74,22 @@ public static String launderRevision(String value) {
7574

7675
/**
7776
* Sanitize {@code value} where it will be used in subsequent OpenGrok
78-
* (non-logging) processing. The value is assumed to represent a file path,
79-
* not necessarily existent on the file system.
80-
* @return {@code null} if null or else {@code value} with path traversal
81-
* path components {@code /../} removed.
77+
* (non-logging) processing. The value is assumed to represent URI path,
78+
* not necessarily existent on the file system. Further, it assumes that the URI path
79+
* is already decoded, e.g. {@code %2e%2e} turned into {@code ..}.
80+
* @return {@code value} with path traversal path components {@code /../}
81+
* and null characters replaced with {@code _}.
8282
*/
83-
public static String launderPath(@NotNull String value) {
84-
Path path = Path.of(Laundromat.launderInput(value));
83+
public static String launderUriPath(@NotNull String value) {
8584
List<String> pathElements = new ArrayList<>();
86-
for (int i = 0; i < path.getNameCount(); i++) {
87-
if (path.getName(i).toString().equals("..")) {
85+
String uriPath = Laundromat.launderInput(value);
86+
for (String pathElement : uriPath.split("/")) {
87+
if (pathElement.isEmpty() || pathElement.equals("..")) {
8888
continue;
8989
}
90-
pathElements.add(path.getName(i).toString());
90+
pathElements.add(pathElement);
9191
}
92-
return (path.isAbsolute() ? "/" : "") + String.join("/", pathElements);
92+
return (uriPath.startsWith("/") ? "/" : "") + String.join("/", pathElements);
9393
}
9494

9595
/**

opengrok-indexer/src/test/java/org/opengrok/indexer/web/LaundromatTest.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,17 +70,19 @@ void testLaunderServerName(Pair<String, String> param) {
7070
assertEquals(param.getLeft(), param.getRight());
7171
}
7272

73-
private static Stream<Pair<String, String>> getParamsForTestLaunderPath() {
74-
return Stream.of(Pair.of("foo", Laundromat.launderPath("../../../foo")),
75-
Pair.of("/foo/bar", Laundromat.launderPath("/foo/../../bar")),
76-
Pair.of("/foo/bar..", Laundromat.launderPath("/foo/bar..")),
77-
Pair.of("..foo/bar", Laundromat.launderPath("..foo/bar")),
78-
Pair.of("/foo/bar.txt", Laundromat.launderPath("/foo/bar.txt")));
73+
private static Stream<Pair<String, String>> getParamsForTestLaunderUriPath() {
74+
return Stream.of(Pair.of("foo", Laundromat.launderUriPath("../../../foo")),
75+
Pair.of("/foo/bar", Laundromat.launderUriPath("/foo/../../bar")),
76+
Pair.of("/foo/bar..", Laundromat.launderUriPath("/foo/bar..")),
77+
Pair.of("/foo/bar", Laundromat.launderUriPath("/foo//bar")),
78+
Pair.of("..foo/bar", Laundromat.launderUriPath("..foo/bar")),
79+
Pair.of("/foo/bar.txt", Laundromat.launderUriPath("/foo/bar.txt")),
80+
Pair.of("/foo/bar_.txt", Laundromat.launderUriPath("/foo/bar\u0000.txt")));
7981
}
8082

8183
@ParameterizedTest
82-
@MethodSource("getParamsForTestLaunderPath")
83-
void testLaunderPath(Pair<String, String> param) {
84+
@MethodSource("getParamsForTestLaunderUriPath")
85+
void testLaunderUriPath(Pair<String, String> param) {
8486
assertEquals(param.getLeft(), param.getRight());
8587
}
8688

opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ private boolean getFileRevision(DiffData data, String context, String[] filepath
247247
if (p != null) {
248248
int j = p.lastIndexOf("@");
249249
if (j != -1) {
250-
filepath[i - 1] = Laundromat.launderPath(p.substring(0, j));
250+
filepath[i - 1] = Laundromat.launderUriPath(p.substring(0, j));
251251
data.rev[i - 1] = Laundromat.launderRevision(p.substring(j + 1));
252252
}
253253
}

0 commit comments

Comments
 (0)