Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ public void move(String noteId,
@Override
public void move(String folderPath, String newFolderPath, AuthenticationInfo subject)
throws IOException {
NotebookPathValidator.rejectTraversalSegments(folderPath);
NotebookPathValidator.rejectTraversalSegments(newFolderPath);
// [ZEPPELIN-4195] newFolderPath parent path maybe not exist
this.fs.tryMkDir(new Path(notebookDir, folderPath.substring(1)).getParent());
this.fs.move(new Path(notebookDir, folderPath.substring(1)),
Expand All @@ -119,6 +121,7 @@ public void remove(String noteId, String notePath, AuthenticationInfo subject)

@Override
public void remove(String folderPath, AuthenticationInfo subject) throws IOException {
NotebookPathValidator.rejectTraversalSegments(folderPath);
if (!this.fs.delete(new Path(notebookDir, folderPath.substring(1)))) {
LOGGER.warn("Fail to remove folder: {}", folderPath);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.zeppelin.notebook.repo;

import java.io.IOException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.regex.Pattern;

/**
* Note-path validation helpers shared by {@link NotebookRepo} implementations
* and the service layer. A {@code final} class with {@code static} methods
* (rather than {@link NotebookRepo} default methods) prevents an
* implementation from accidentally — or intentionally — overriding the
* checks and bypassing them.
*/
public final class NotebookPathValidator {

private static final Pattern PATH_SEGMENT_SPLIT = Pattern.compile("/+");
private static final String PARENT_SEGMENT = "..";
private static final String CURRENT_SEGMENT = ".";
private static final int MAX_DECODE_LAYERS = 5;

private NotebookPathValidator() {
}

/**
* Refuses any {@code ..} or {@code .} segment in {@code notePath}. The
* input is URL-decoded repeatedly first so that variants such as
* {@code %2e%2e} or {@code %252e%252e} cannot bypass the check.
*
* @throws IOException if the path is null, contains a traversal segment,
* or has more URL-encoding layers than {@value #MAX_DECODE_LAYERS}
*/
public static void rejectTraversalSegments(String notePath) throws IOException {
if (notePath == null) {
throw new IOException("Path must not be null");
}
String decoded = decodeRepeatedly(notePath);
String stripped = decoded.startsWith("/") ? decoded.substring(1) : decoded;
for (String segment : PATH_SEGMENT_SPLIT.split(stripped)) {
if (PARENT_SEGMENT.equals(segment) || CURRENT_SEGMENT.equals(segment)) {
throw new IOException("Path traversal segments are not allowed: " + notePath);
}
}
}

/**
* Repeatedly URL-decodes {@code encoded} until it stabilises, capped at
* {@value #MAX_DECODE_LAYERS} layers.
*/
public static String decodeRepeatedly(String encoded) throws IOException {
String previous = encoded;
for (int attempt = 0; attempt < MAX_DECODE_LAYERS; attempt++) {
String decoded = URLDecoder.decode(previous, StandardCharsets.UTF_8);
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 @@ -146,6 +146,7 @@ default String buildNoteFileName(String noteId, String notePath) throws IOExcept
if (!notePath.startsWith("/")) {
throw new IOException("Invalid notePath: " + notePath);
}
NotebookPathValidator.rejectTraversalSegments(notePath);
return (notePath + "_" + noteId + ".zpln").substring(1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.Instant;
Expand Down Expand Up @@ -57,6 +55,7 @@
import org.apache.zeppelin.notebook.AuthorizationService;
import org.apache.zeppelin.notebook.exception.CorruptedNoteException;
import org.apache.zeppelin.notebook.exception.NotePathAlreadyExistsException;
import org.apache.zeppelin.notebook.repo.NotebookPathValidator;
import org.apache.zeppelin.notebook.repo.NotebookRepoWithVersionControl;
import org.apache.zeppelin.notebook.scheduler.SchedulerService;
import org.apache.zeppelin.common.Message;
Expand Down Expand Up @@ -240,7 +239,7 @@ String normalizeNotePath(String notePath) throws IOException {

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

notePath = decodeRepeatedly(notePath);
notePath = NotebookPathValidator.decodeRepeatedly(notePath);
if (notePath.endsWith("/")) {
throw new IOException("Note name shouldn't end with '/'");
}
Expand Down Expand Up @@ -315,6 +314,7 @@ public void renameNote(String noteId,
}
}
try {
newNotePathReal = normalizeNotePath(newNotePathReal);
notebook.moveNote(noteId, newNotePathReal, context.getAutheInfo());
callback.onSuccess(readNote, context);
} catch (NotePathAlreadyExistsException e) {
Expand Down Expand Up @@ -1567,20 +1567,4 @@ private <T> boolean checkPermission(String noteId,
}
}

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);
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
@@ -0,0 +1,97 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.zeppelin.notebook.repo;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.io.IOException;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

class NotebookRepoPathValidationTest {

@ParameterizedTest
@ValueSource(strings = {
"/../etc/passwd",
"/foo/../../etc/passwd",
"/foo/../bar",
"/foo/./bar",
"/./bar",
"/foo/..",
"/..",
"/.",
// URL-encoded variants must be rejected after decoding.
"/%2e%2e/etc/passwd",
"/foo/%2E%2E/bar",
"/%2e/foo",
// Double-encoded.
"/%252e%252e/etc/passwd",
})
void rejectTraversalSegments_rejects_traversal(String malicious) {
assertThrows(IOException.class,
() -> NotebookPathValidator.rejectTraversalSegments(malicious),
"expected rejection for: " + malicious);
}

@ParameterizedTest
@ValueSource(strings = {
"/MyNote",
"/folder/MyNote",
"/folder/sub-folder/My Note With Spaces",
"/한글노트",
"/foo.bar.baz",
// ".." and "." are rejected only as exact segments — names containing
// dots remain valid.
"/...",
"/foo/...",
"/foo..bar",
})
void rejectTraversalSegments_accepts_normal_paths(String safe) throws IOException {
NotebookPathValidator.rejectTraversalSegments(safe);
}

@Test
void rejectTraversalSegments_rejects_null() {
assertThrows(IOException.class, () -> NotebookPathValidator.rejectTraversalSegments(null));
}

@Test
void rejectTraversalSegments_rejects_excessive_encoding_layers() {
// Six layers of "..", past the 5-layer decode cap.
String payload = "/%252525252e%252525252e/etc";
assertThrows(IOException.class, () -> NotebookPathValidator.rejectTraversalSegments(payload));
}

@Test
void decodeRepeatedly_returns_input_when_already_decoded() throws IOException {
assertEquals("/foo bar", NotebookPathValidator.decodeRepeatedly("/foo bar"));
}

@Test
void decodeRepeatedly_decodes_until_stable() throws IOException {
assertEquals("/..", NotebookPathValidator.decodeRepeatedly("/%252e%252e"));
}

@Test
void decodeRepeatedly_throws_on_too_many_layers() {
String payload = "/%2525252525252525252e";
assertThrows(IOException.class, () -> NotebookPathValidator.decodeRepeatedly(payload));
}
}
Loading