From 575f123e9c83e45f14e28009a7fdc5c5c1a0def7 Mon Sep 17 00:00:00 2001 From: Chan Ho Lee Date: Sun, 3 Nov 2024 21:36:31 +0900 Subject: [PATCH 1/2] Prevent directory escape bypass through repeated URL decoding --- .../zeppelin/service/NotebookService.java | 20 ++++++++++++++++++- .../zeppelin/service/NotebookServiceTest.java | 13 ++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java index a0c8908d60e..b5619ed3640 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java @@ -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; @@ -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 '/'"); } @@ -1553,4 +1554,21 @@ private 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."); + } } diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java index be30a3cda89..152d0856688 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/NotebookServiceTest.java @@ -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"); From 38780496f432f46f15e6a52cd80f56c10600623d Mon Sep 17 00:00:00 2001 From: Chan Ho Lee Date: Sun, 24 Nov 2024 21:12:30 +0900 Subject: [PATCH 2/2] Reflecting review feedback --- .../java/org/apache/zeppelin/service/NotebookService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java index b5619ed3640..e22a2c1a7f9 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java @@ -1555,13 +1555,13 @@ private boolean checkPermission(String noteId, } } - private String decodeRepeatedly(final String encoded) throws IOException { + private static 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()); + String decoded = URLDecoder.decode(previous, StandardCharsets.UTF_8); attempts++; if (decoded.equals(previous)) { return decoded;