Caideyipi commented on code in PR #18488:
URL: https://github.com/apache/iotdb/pull/18488#discussion_r3840496706
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/meta/PipePluginMeta.java:
##########
@@ -195,4 +201,13 @@ public String toString() {
+ '\''
+ '}';
}
+
+ private static String validatePathSegment(final String pathSegment) {
+ final String pathError = FileUtils.getIllegalError4Directory(pathSegment);
+ if (pathError != null) {
+ throw new IllegalArgumentException(pathError);
+ }
+ Paths.get(pathSegment);
+ return pathSegment;
Review Comment:
Applied in 5a3d16f18d6. The line was intended as a platform-level syntax
check: after the application-level segment validation, Paths.get(pathSegment)
rejects provider-specific invalid input such as NUL characters. I moved this
check into the shared FileUtils.validatePathSegment helper so the purpose is
explicit and the validation is reused.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManager.java:
##########
@@ -138,7 +132,48 @@ public void linkExistedPlugin(
*/
public void savePluginToInstallDir(ByteBuffer byteBuffer, String pluginName,
String fileName)
throws IOException {
- String destination = getPluginInstallPathV2(pluginName, fileName);
- saveToDir(byteBuffer, destination);
+ saveToDir(byteBuffer, getPluginInstallPathV2Path(pluginName,
fileName).toString());
+ }
+
+ private Path getPluginInstallPathV2Path(final String pluginName, final
String fileName) {
+ return resolvePathUnderDirectory(getPluginDirectoryPath(pluginName),
fileName);
+ }
+
+ private Path getPluginDirectoryPath(final String pluginName) {
+ final String validatedPluginName = validatePathSegment(pluginName);
+ return resolvePathUnderDirectory(
+ getInstallDirectoryPath(),
validatedPluginName.toUpperCase(Locale.ROOT));
+ }
+
+ private Path getInstallDirectoryPath() {
+ return Paths.get(libRoot, INSTALL_DIR).toAbsolutePath().normalize();
+ }
+
+ /**
+ * Resolves a single untrusted path segment below {@code baseDirectory}.
+ *
+ * <p>The segment validation rejects separators and dot segments, while the
normalized containment
+ * check remains as a defense in depth for absolute paths and future callers.
+ */
+ private Path resolvePathUnderDirectory(final Path baseDirectory, final
String pathSegment) {
+ validatePathSegment(pathSegment);
+
+ final Path normalizedBaseDirectory =
baseDirectory.toAbsolutePath().normalize();
+ final Path normalizedTargetPath =
+
normalizedBaseDirectory.resolve(pathSegment).toAbsolutePath().normalize();
+ if (!normalizedTargetPath.startsWith(normalizedBaseDirectory)) {
+ throw new IllegalArgumentException(
+ String.format(PipeMessages.ILLEGAL_FILENAME_PATH_TRAVERSAL,
pathSegment));
+ }
+ return normalizedTargetPath;
+ }
+
+ private String validatePathSegment(final String pathSegment) {
+ final String pathError = FileUtils.getIllegalError4Directory(pathSegment);
+ if (pathError != null) {
+ throw new IllegalArgumentException(pathError);
+ }
+ Paths.get(pathSegment);
+ return pathSegment;
}
Review Comment:
Applied in 5a3d16f18d6. Removed the duplicate validatePathSegment
implementation from PipePluginExecutableManager and reused the shared
FileUtils.validatePathSegment helper; all plugin path construction and metadata
validation now go through the same check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
