Skip to content

Commit

Permalink
Prevent directory escape bypass through repeated URL decoding
Browse files Browse the repository at this point in the history
  • Loading branch information
tbonelee committed Nov 3, 2024
1 parent 3575a3c commit 575f123
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.apache.zeppelin.scheduler.Job.Status.ABORT;

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
Expand Down Expand Up @@ -239,7 +240,7 @@ String normalizeNotePath(String notePath) throws IOException {

notePath = notePath.replace("\r", " ").replace("\n", " ");

notePath = URLDecoder.decode(notePath, StandardCharsets.UTF_8.toString());
notePath = decodeRepeatedly(notePath);
if (notePath.endsWith("/")) {
throw new IOException("Note name shouldn't end with '/'");
}
Expand Down Expand Up @@ -1553,4 +1554,21 @@ private <T> boolean checkPermission(String noteId,
return false;
}
}

private String decodeRepeatedly(final String encoded) throws IOException {
String previous = encoded;
int maxDecodeAttempts = 5;
int attempts = 0;

while (attempts < maxDecodeAttempts) {
String decoded = URLDecoder.decode(previous, StandardCharsets.UTF_8.toString());
attempts++;
if (decoded.equals(previous)) {
return decoded;
}
previous = decoded;
}

throw new IOException("Exceeded maximum decode attempts. Possible malicious input.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,19 @@ void testNormalizeNotePath() throws IOException {
} catch (IOException e) {
assertEquals("Note name can not contain '..'", e.getMessage());
}
try {
// Double URL encoding of ".."
notebookService.normalizeNotePath("%252e%252e/%252e%252e/tmp/test333");
fail("Should fail");
} catch (IOException e) {
assertEquals("Note name can not contain '..'", e.getMessage());
}
try {
notebookService.normalizeNotePath("%252525252e%252525252e/tmp/test444");
fail("Should fail");
} catch (IOException e) {
assertEquals("Exceeded maximum decode attempts. Possible malicious input.", e.getMessage());
}
try {
notebookService.normalizeNotePath("./");
fail("Should fail");
Expand Down

0 comments on commit 575f123

Please sign in to comment.