This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new e1057ac26dc [branch-2.1][fix](metadata)Add FE metadata-related file
checks #40546 (#41113)
e1057ac26dc is described below
commit e1057ac26dc9a84aa1a0c2c6bc5bc67873cc90ba
Author: Calvin Kirs <[email protected]>
AuthorDate: Mon Sep 23 17:13:35 2024 +0800
[branch-2.1][fix](metadata)Add FE metadata-related file checks #40546
(#41113)
## Proposed changes
#40546
---
.../main/java/org/apache/doris/common/Config.java | 4 +
.../java/org/apache/doris/master/MetaHelper.java | 98 +++++++++++++++++++---
.../org/apache/doris/master/MetaHelperTest.java | 59 +++++++++++++
3 files changed, 150 insertions(+), 11 deletions(-)
diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
index 3bf55f3da40..c9a83d93146 100644
--- a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
+++ b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
@@ -2806,4 +2806,8 @@ public class Config extends ConfigBase {
@ConfField(mutable = true, description = {"表示最大锁持有时间，超过该时间会打印告警日志，单位秒",
"Maximum lock hold time; logs a warning if exceeded"})
public static long max_lock_hold_threshold_seconds = 10;
+
+ @ConfField(mutable = true, description = {"元数据同步是否开启安全模式",
+ "Is metadata synchronization enabled in safe mode"})
+ public static boolean meta_helper_security_mode = false;
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java
b/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java
index fe516b02bfd..e95a7a1cebc 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java
@@ -18,6 +18,7 @@
package org.apache.doris.master;
import org.apache.doris.catalog.Env;
+import org.apache.doris.common.Config;
import org.apache.doris.common.io.IOUtils;
import org.apache.doris.common.util.HttpURLUtil;
import org.apache.doris.httpv2.entity.ResponseBody;
@@ -25,24 +26,29 @@ import org.apache.doris.httpv2.rest.manager.HttpUtils;
import org.apache.doris.persist.gson.GsonUtils;
import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import java.io.BufferedInputStream;
import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
-import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.HttpURLConnection;
+import java.util.Map;
public class MetaHelper {
+ public static final Logger LOG = LogManager.getLogger(MetaHelper.class);
private static final String PART_SUFFIX = ".part";
public static final String X_IMAGE_SIZE = "X-Image-Size";
public static final String X_IMAGE_MD5 = "X-Image-Md5";
private static final int BUFFER_BYTES = 8 * 1024;
private static final int CHECKPOINT_LIMIT_BYTES = 30 * 1024 * 1024;
+ private static final String VALID_FILENAME_REGEX =
"^(?!\\.)[a-zA-Z0-9_\\-.]+$";
public static File getMasterImageDir() {
String metaDir = Env.getCurrentEnv().getImageDir();
@@ -53,28 +59,97 @@ public class MetaHelper {
return CHECKPOINT_LIMIT_BYTES;
}
+ private static void completeCheck(File dir, File file, File newFile)
throws IOException {
+ if (!Config.meta_helper_security_mode) {
+ return;
+ }
+ String dirPath = dir.getCanonicalPath(); // Get the canonical path of
the directory
+ String filePath = file.getCanonicalPath(); // Get the canonical path
of the original file
+ String newFilePath = newFile.getCanonicalPath(); // Get the canonical
path of the new file
+
+ // Ensure both file paths are within the specified directory to
prevent path traversal attacks
+ if (!filePath.startsWith(dirPath) || !newFilePath.startsWith(dirPath))
{
+ throw new SecurityException("File path traversal attempt
detected.");
+ }
+
+ // Ensure the original file exists and is a valid file to avoid
renaming a non-existing file
+ if (!file.exists() || !file.isFile()) {
+ throw new IOException("Source file does not exist or is not a
valid file.");
+ }
+
+ }
+
// rename the .PART_SUFFIX file to filename
public static File complete(String filename, File dir) throws IOException {
- File file = new File(dir, filename + MetaHelper.PART_SUFFIX);
- File newFile = new File(dir, filename);
+ // Validate that the filename does not contain illegal path elements
+ checkIsValidFileName(filename);
+
+ File file = new File(dir, filename + MetaHelper.PART_SUFFIX); //
Original file with a specific suffix
+ File newFile = new File(dir, filename); // Target file without the
suffix
+
+ completeCheck(dir, file, newFile);
+ // Attempt to rename the file. If it fails, throw an exception
if (!file.renameTo(newFile)) {
- throw new IOException("Complete file" + filename + " failed");
+ throw new IOException("Complete file " + filename + " failed");
}
- return newFile;
+
+ return newFile; // Return the newly renamed file
}
- public static OutputStream getOutputStream(String filename, File dir)
- throws FileNotFoundException {
+ public static File getFile(String filename, File dir) throws IOException {
+ checkIsValidFileName(filename);
File file = new File(dir, filename + MetaHelper.PART_SUFFIX);
- return new FileOutputStream(file);
+ checkFile(dir, file);
+ return file;
+ }
+
+ private static void checkFile(File dir, File file) throws IOException {
+ if (!Config.meta_helper_security_mode) {
+ return;
+ }
+ String dirPath = dir.getCanonicalPath();
+ String filePath = file.getCanonicalPath();
+
+ if (!filePath.startsWith(dirPath)) {
+ throw new SecurityException("File path traversal attempt
detected.");
+ }
+ }
+
+ protected static void checkIsValidFileName(String filename) {
+ if (!Config.meta_helper_security_mode) {
+ return;
+ }
+ if (StringUtils.isBlank(filename)) {
+ return;
+ }
+ if (!filename.matches(VALID_FILENAME_REGEX)) {
+ throw new IllegalArgumentException("Invalid filename : " +
filename);
+ }
}
- public static File getFile(String filename, File dir) {
- return new File(dir, filename + MetaHelper.PART_SUFFIX);
+ private static void checkFile(File file) throws IOException {
+ if (!Config.meta_helper_security_mode) {
+ return;
+ }
+ if
(!file.getAbsolutePath().startsWith(file.getCanonicalFile().getParent())) {
+ throw new IllegalArgumentException("Invalid file path");
+ }
+
+ File parentDir = file.getParentFile();
+ if (!parentDir.canWrite()) {
+ throw new IOException("No write permission in directory: " +
parentDir);
+ }
+
+ if (file.exists() && !file.delete()) {
+ throw new IOException("Failed to delete existing file: " + file);
+ }
+ checkIsValidFileName(file.getName());
}
public static <T> ResponseBody doGet(String url, int timeout, Class<T>
clazz) throws IOException {
- String response = HttpUtils.doGet(url,
HttpURLUtil.getNodeIdentHeaders(), timeout);
+ Map<String, String> headers = HttpURLUtil.getNodeIdentHeaders();
+ LOG.info("meta helper, url: {}, timeout{}, headers: {}", url, timeout,
headers);
+ String response = HttpUtils.doGet(url, headers, timeout);
return parseResponse(response, clazz);
}
@@ -82,6 +157,7 @@ public class MetaHelper {
public static void getRemoteFile(String urlStr, int timeout, File file)
throws IOException {
HttpURLConnection conn = null;
+ checkFile(file);
OutputStream out = new FileOutputStream(file);
try {
conn = HttpURLUtil.getConnectionWithNodeIdent(urlStr);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java
b/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java
index 070979494bf..1c8c8a2a7dd 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java
@@ -17,6 +17,7 @@
package org.apache.doris.master;
+import org.apache.doris.common.Config;
import org.apache.doris.httpv2.entity.ResponseBody;
import org.apache.doris.httpv2.rest.RestApiStatusCode;
import org.apache.doris.persist.StorageInfo;
@@ -25,6 +26,11 @@ import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Assert;
import org.junit.Test;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+
+import java.io.File;
+import java.io.IOException;
public class MetaHelperTest {
@@ -49,4 +55,57 @@ public class MetaHelperTest {
bodyBefore.setMsg("msg");
return bodyBefore;
}
+
+ File tempDir = new File(System.getProperty("java.io.tmpdir"), "tempDir");
+
+ @BeforeEach
+ void setUp() {
+
+ if (tempDir.exists()) {
+ tempDir.delete();
+ }
+ tempDir.mkdir();
+ }
+
+ @Test
+ public void testFile() throws IOException {
+
+ String errorFilename = "..testfile.";
+ File errorFileWithSuffix = new File(tempDir, errorFilename);
+ String rightFilename = "image.1";
+ File rightFileWithSuffix = new File(tempDir, rightFilename);
+
+ Config.meta_helper_security_mode = true;
+
+ if (errorFileWithSuffix.exists()) {
+ errorFileWithSuffix.delete();
+ }
+ Assert.assertThrows(Exception.class, () ->
MetaHelper.complete(errorFilename, tempDir));
+ Assert.assertThrows(Exception.class, () ->
MetaHelper.getFile(errorFilename, tempDir));
+ if (rightFileWithSuffix.exists()) {
+ rightFileWithSuffix.delete();
+ }
+ Assert.assertEquals(rightFileWithSuffix.getName() + ".part",
MetaHelper.getFile(rightFilename, tempDir).getName());
+
+ }
+
+ @Test
+ public void testFileNameCheck() {
+ Config.meta_helper_security_mode = true;
+ MetaHelper.checkIsValidFileName("VERSION");
+ MetaHelper.checkIsValidFileName("image.1");
+ MetaHelper.checkIsValidFileName("image.1.part");
+ MetaHelper.checkIsValidFileName("image.1.part.1");
+ Assert.assertThrows(IllegalArgumentException.class, () ->
MetaHelper.checkIsValidFileName("../testfile."));
+
+
+ }
+
+ @AfterEach
+ public void tearDown() {
+ if (tempDir.exists()) {
+ tempDir.delete();
+ }
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
