Skip to content

Commit b561010

Browse files
XComprmetzger
XComp
authored and
rmetzger
committed
[hotfix][runtime] It was possible to traverse the directory of the host through /jobmanager/logs/<path-to-file>.
The <path-to-file> had to be modified in a way that '../' referring to the parent folder needed to be replaced by '..%252f'. This enabled traversing the directory structure relative to the ./logs folder, e.g. /jobmanager/logs/..%252f/README.txt would return the content of the README.txt located in the FLINK_HOME folder (assuming Flink's default folder structure). This is fixed now. The passed path is ignored in the same way as it's already done for the TaskManagerCustomLogHandler.
1 parent 7fed9f0 commit b561010

File tree

2 files changed

+165
-1
lines changed

2 files changed

+165
-1
lines changed

‎flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/cluster/JobManagerCustomLogHandler.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ protected File getFile(HandlerRequest<EmptyRequestBody, FileMessageParameters> h
5454
if (logDir == null) {
5555
return null;
5656
}
57-
String filename = handlerRequest.getPathParameter(LogFileNamePathParameter.class);
57+
// wrapping around another File instantiation is a simple way to remove any path information - we're
58+
// solely interested in the filename
59+
String filename = new File(handlerRequest.getPathParameter(LogFileNamePathParameter.class)).getName();
5860
return new File(logDir, filename);
5961
}
6062
}

‎flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/cluster/JobManagerCustomLogHandlerTest.java

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.flink.runtime.rest.handler.cluster;
20+
21+
import org.apache.flink.runtime.rest.handler.HandlerRequest;
22+
import org.apache.flink.runtime.rest.handler.HandlerRequestException;
23+
import org.apache.flink.runtime.rest.messages.EmptyRequestBody;
24+
import org.apache.flink.runtime.rest.messages.cluster.FileMessageParameters;
25+
import org.apache.flink.runtime.rest.messages.cluster.JobManagerCustomLogHeaders;
26+
import org.apache.flink.runtime.testingUtils.TestingUtils;
27+
import org.apache.flink.runtime.webmonitor.TestingDispatcherGateway;
28+
import org.apache.flink.util.TestLogger;
29+
30+
import org.apache.commons.io.FileUtils;
31+
import org.junit.Before;
32+
import org.junit.Rule;
33+
import org.junit.Test;
34+
import org.junit.rules.TemporaryFolder;
35+
36+
import java.io.File;
37+
import java.io.IOException;
38+
import java.nio.charset.StandardCharsets;
39+
import java.nio.file.Files;
40+
import java.util.Collections;
41+
import java.util.HashMap;
42+
import java.util.Map;
43+
import java.util.concurrent.CompletableFuture;
44+
45+
import static org.hamcrest.Matchers.is;
46+
import static org.hamcrest.Matchers.notNullValue;
47+
import static org.junit.Assert.assertFalse;
48+
import static org.junit.Assert.assertThat;
49+
import static org.junit.Assert.assertTrue;
50+
51+
/**
52+
* Unit tests for {@link JobManagerCustomLogHandler}.
53+
*/
54+
public class JobManagerCustomLogHandlerTest extends TestLogger {
55+
56+
private static final String FORBIDDEN_FILENAME = "forbidden";
57+
58+
private static final String VALID_LOG_FILENAME = "valid.log";
59+
private static final String VALID_LOG_CONTENT = "logged content";
60+
61+
@Rule
62+
public TemporaryFolder temporaryFolder = new TemporaryFolder();
63+
64+
private File logRoot;
65+
66+
private JobManagerCustomLogHandler testInstance;
67+
68+
@Before
69+
public void setUp() throws IOException {
70+
initializeFolderStructure();
71+
72+
final TestingDispatcherGateway dispatcherGateway = new TestingDispatcherGateway.Builder().build();
73+
testInstance = new JobManagerCustomLogHandler(
74+
() -> CompletableFuture.completedFuture(dispatcherGateway),
75+
TestingUtils.TIMEOUT(),
76+
Collections.emptyMap(),
77+
JobManagerCustomLogHeaders.getInstance(),
78+
logRoot);
79+
}
80+
81+
private void initializeFolderStructure() throws IOException {
82+
File root = temporaryFolder.getRoot();
83+
logRoot = new File(root, "logs");
84+
assertTrue(logRoot.mkdir());
85+
86+
createFile(new File(root, FORBIDDEN_FILENAME), "forbidden content");
87+
createFile(new File(logRoot, VALID_LOG_FILENAME), VALID_LOG_CONTENT);
88+
}
89+
90+
private static void createFile(File file, String content) throws IOException {
91+
FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8);
92+
}
93+
94+
private static HandlerRequest<EmptyRequestBody, FileMessageParameters> createHandlerRequest(String path) throws HandlerRequestException {
95+
FileMessageParameters messageParameters = new FileMessageParameters();
96+
Map<String, String> pathParameters = new HashMap<>();
97+
pathParameters.put(messageParameters.logFileNamePathParameter.getKey(), path);
98+
99+
return new HandlerRequest<>(
100+
EmptyRequestBody.getInstance(),
101+
messageParameters,
102+
pathParameters,
103+
Collections.emptyMap());
104+
}
105+
106+
@Test
107+
public void testGetJobManagerCustomLogsValidFilename() throws Exception {
108+
File actualFile = testInstance.getFile(createHandlerRequest(VALID_LOG_FILENAME));
109+
assertThat(actualFile, is(notNullValue()));
110+
111+
String actualContent = String.join("", Files.readAllLines(actualFile.toPath()));
112+
assertThat(actualContent, is(VALID_LOG_CONTENT));
113+
}
114+
115+
@Test
116+
public void testGetJobManagerCustomLogsValidFilenameWithPath() throws Exception {
117+
File actualFile = testInstance.getFile(createHandlerRequest(String.format("foobar/%s", VALID_LOG_FILENAME)));
118+
assertThat(actualFile, is(notNullValue()));
119+
120+
String actualContent = String.join("", Files.readAllLines(actualFile.toPath()));
121+
assertThat(actualContent, is(VALID_LOG_CONTENT));
122+
}
123+
124+
@Test
125+
public void testGetJobManagerCustomLogsValidFilenameWithInvalidPath() throws Exception {
126+
File actualFile = testInstance.getFile(createHandlerRequest(String.format("../%s", VALID_LOG_FILENAME)));
127+
assertThat(actualFile, is(notNullValue()));
128+
129+
String actualContent = String.join("", Files.readAllLines(actualFile.toPath()));
130+
assertThat(actualContent, is(VALID_LOG_CONTENT));
131+
}
132+
133+
@Test
134+
public void testGetJobManagerCustomLogsNotExistingFile() throws Exception {
135+
File actualFile = testInstance.getFile(createHandlerRequest("not-existing"));
136+
assertThat(actualFile, is(notNullValue()));
137+
assertFalse(actualFile.exists());
138+
}
139+
140+
@Test
141+
public void testGetJobManagerCustomLogsExistingButForbiddenFile() throws Exception {
142+
File actualFile = testInstance.getFile(createHandlerRequest(String.format("../%s", FORBIDDEN_FILENAME)));
143+
assertThat(actualFile, is(notNullValue()));
144+
assertFalse(actualFile.exists());
145+
}
146+
147+
@Test
148+
public void testGetJobManagerCustomLogsExistingButForbiddenFileWithObfuscatedPath() throws Exception {
149+
File actualFile = testInstance.getFile(createHandlerRequest(String.format("..%%252%s", FORBIDDEN_FILENAME)));
150+
assertThat(actualFile, is(notNullValue()));
151+
assertFalse(actualFile.exists());
152+
}
153+
154+
@Test
155+
public void testGetJobManagerCustomLogsValidFilenameWithLongInvalidPath() throws Exception {
156+
File actualFile = testInstance.getFile(createHandlerRequest(String.format("foobar/../../%s", VALID_LOG_FILENAME)));
157+
assertThat(actualFile, is(notNullValue()));
158+
159+
String actualContent = String.join("", Files.readAllLines(actualFile.toPath()));
160+
assertThat(actualContent, is(VALID_LOG_CONTENT));
161+
}
162+
}

0 commit comments

Comments
 (0)