Skip to content

Commit 588b54b

Browse files
authored
[ZEPPELIN-6190] Prevent directory escape bypass through repeated URL decoding
### What is this PR for? This PR addresses an issue in `NotebookService` where the notebook path validation only performs a single decoding pass. This allowed a malicious user to bypass validation by double-encoding the `".."` token. By implementing the repeated decoding, we can prevent this bypass. Additionally, to prevent excessive decoding attempts, a maximum limit on the number of decoding attempts has been added. ### What type of PR is it? Hot Fix ### What is the Jira issue? https://issues.apache.org/jira/projects/ZEPPELIN/issues/ZEPPELIN-6190 ### How should this be tested? * CI ### Questions: * Does the license files need to update? No * Is there breaking changes for older versions? * There may be minor compatibility issues if a user relies on multiple encoded paths, but this is unlikely in realistic scenarios. * Does this needs documentation? No Closes #4891 from tbonelee/fix-validating-note-path. Signed-off-by: Philipp Dallig <[email protected]>
1 parent 9e35613 commit 588b54b

File tree

2 files changed

+32
-1
lines changed

2 files changed

+32
-1
lines changed

zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.apache.zeppelin.scheduler.Job.Status.ABORT;
2525

2626
import java.io.IOException;
27+
import java.io.UnsupportedEncodingException;
2728
import java.net.URLDecoder;
2829
import java.nio.charset.StandardCharsets;
2930
import java.text.ParseException;
@@ -239,7 +240,7 @@ String normalizeNotePath(String notePath) throws IOException {
239240

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

242-
notePath = URLDecoder.decode(notePath, StandardCharsets.UTF_8.toString());
243+
notePath = decodeRepeatedly(notePath);
243244
if (notePath.endsWith("/")) {
244245
throw new IOException("Note name shouldn't end with '/'");
245246
}
@@ -1563,4 +1564,21 @@ private <T> boolean checkPermission(String noteId,
15631564
return false;
15641565
}
15651566
}
1567+
1568+
private static String decodeRepeatedly(final String encoded) throws IOException {
1569+
String previous = encoded;
1570+
int maxDecodeAttempts = 5;
1571+
int attempts = 0;
1572+
1573+
while (attempts < maxDecodeAttempts) {
1574+
String decoded = URLDecoder.decode(previous, StandardCharsets.UTF_8);
1575+
attempts++;
1576+
if (decoded.equals(previous)) {
1577+
return decoded;
1578+
}
1579+
previous = decoded;
1580+
}
1581+
1582+
throw new IOException("Exceeded maximum decode attempts. Possible malicious input.");
1583+
}
15661584
}

zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,19 @@ void testNormalizeNotePath() throws IOException {
615615
} catch (IOException e) {
616616
assertEquals("Note name can not contain '..'", e.getMessage());
617617
}
618+
try {
619+
// Double URL encoding of ".."
620+
notebookService.normalizeNotePath("%252e%252e/%252e%252e/tmp/test333");
621+
fail("Should fail");
622+
} catch (IOException e) {
623+
assertEquals("Note name can not contain '..'", e.getMessage());
624+
}
625+
try {
626+
notebookService.normalizeNotePath("%252525252e%252525252e/tmp/test444");
627+
fail("Should fail");
628+
} catch (IOException e) {
629+
assertEquals("Exceeded maximum decode attempts. Possible malicious input.", e.getMessage());
630+
}
618631
try {
619632
notebookService.normalizeNotePath("./");
620633
fail("Should fail");

0 commit comments

Comments
 (0)