github-advanced-security[bot] commented on code in PR #7940:
URL: https://github.com/apache/geode/pull/7940#discussion_r2433256309
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java:
##########
@@ -251,4 +294,126 @@
return result;
}
}
+
+ /**
+ * Security: Validates JAR file paths to prevent path injection attacks.
+ *
+ * This method addresses CodeQL vulnerability java/path-injection by ensuring
+ * that user-provided file paths are safe to access and don't contain
malicious
+ * path traversal sequences.
+ *
+ * SECURITY ENHANCEMENTS:
+ * 1. Pre-validation of path strings before File object creation
+ * 2. Canonical path validation to prevent sophisticated traversal attacks
+ * 3. System directory access prevention (Linux and Windows)
+ * 4. Enhanced path traversal detection with multiple patterns
+ * 5. File type validation and accessibility checks
+ *
+ * COMPLIANCE:
+ * - Fixes CodeQL vulnerability: java/path-injection
+ * - Follows OWASP path traversal prevention guidelines
+ * - Implements defense-in-depth security validation
+ *
+ * @param jarPath The JAR file path to validate
+ * @throws IllegalArgumentException if the path is invalid or unsafe
+ */
+ private void validateJarPath(String jarPath) {
+ if (jarPath == null || jarPath.trim().isEmpty()) {
+ throw new IllegalArgumentException("JAR file path cannot be null or
empty");
+ }
+
+ // Security: Normalize and validate the path string before creating File
object
+ String normalizedPath = jarPath.trim();
+
+ // Security: Prevent path traversal attacks - check for dangerous patterns
+ if (normalizedPath.contains("..") || normalizedPath.contains("~") ||
+ normalizedPath.contains("\\..") || normalizedPath.contains("/..")) {
+ throw new IllegalArgumentException("Invalid JAR file path: path
traversal detected");
+ }
+
+ // Security: Prevent absolute paths to system directories
+ if (normalizedPath.startsWith("/etc/") ||
normalizedPath.startsWith("/sys/") ||
+ normalizedPath.startsWith("/proc/") ||
normalizedPath.startsWith("/dev/") ||
+ normalizedPath.contains(":\\Windows\\") ||
normalizedPath.contains(":\\Program Files\\")) {
+ throw new IllegalArgumentException("Access to system directories is not
allowed");
+ }
+
+ File jarFile;
+ try {
+ // Security: Create File object and immediately get canonical path for
validation
+ jarFile = new File(normalizedPath);
+ String canonicalPath = jarFile.getCanonicalPath();
+
+ // Security: Ensure canonical path doesn't escape intended directory
bounds
+ // This prevents sophisticated path traversal attacks that might bypass
simple string checks
+ if (!canonicalPath.equals(jarFile.getAbsolutePath())) {
+ // Check if the canonical path contains suspicious path traversal
elements
+ // Security: Use the original normalized path for validation instead
of jarFile.getName()
+ String expectedFileName = new File(normalizedPath).getName();
+ if (canonicalPath.contains("..") ||
!canonicalPath.endsWith(expectedFileName)) {
+ throw new IllegalArgumentException(
+ "Invalid JAR file path: canonical path validation failed");
+ }
+ }
+ } catch (java.io.IOException e) {
+ throw new IllegalArgumentException("Invalid JAR file path: " +
e.getMessage());
+ }
+
+ // Security: Ensure the file exists and is a regular file
+ if (!jarFile.exists()) {
+ // Security: Use sanitized filename in error message to prevent
information disclosure
+ String safeFileName = sanitizeFilename(jarFile.getName());
+ throw new IllegalArgumentException("JAR file does not exist: " +
safeFileName);
+ }
+
+ if (!jarFile.isFile()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
This path depends on a [user-provided value](2).
[Show more
details](https://github.com/apache/geode/security/code-scanning/131)
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java:
##########
@@ -251,4 +294,126 @@
return result;
}
}
+
+ /**
+ * Security: Validates JAR file paths to prevent path injection attacks.
+ *
+ * This method addresses CodeQL vulnerability java/path-injection by ensuring
+ * that user-provided file paths are safe to access and don't contain
malicious
+ * path traversal sequences.
+ *
+ * SECURITY ENHANCEMENTS:
+ * 1. Pre-validation of path strings before File object creation
+ * 2. Canonical path validation to prevent sophisticated traversal attacks
+ * 3. System directory access prevention (Linux and Windows)
+ * 4. Enhanced path traversal detection with multiple patterns
+ * 5. File type validation and accessibility checks
+ *
+ * COMPLIANCE:
+ * - Fixes CodeQL vulnerability: java/path-injection
+ * - Follows OWASP path traversal prevention guidelines
+ * - Implements defense-in-depth security validation
+ *
+ * @param jarPath The JAR file path to validate
+ * @throws IllegalArgumentException if the path is invalid or unsafe
+ */
+ private void validateJarPath(String jarPath) {
+ if (jarPath == null || jarPath.trim().isEmpty()) {
+ throw new IllegalArgumentException("JAR file path cannot be null or
empty");
+ }
+
+ // Security: Normalize and validate the path string before creating File
object
+ String normalizedPath = jarPath.trim();
+
+ // Security: Prevent path traversal attacks - check for dangerous patterns
+ if (normalizedPath.contains("..") || normalizedPath.contains("~") ||
+ normalizedPath.contains("\\..") || normalizedPath.contains("/..")) {
+ throw new IllegalArgumentException("Invalid JAR file path: path
traversal detected");
+ }
+
+ // Security: Prevent absolute paths to system directories
+ if (normalizedPath.startsWith("/etc/") ||
normalizedPath.startsWith("/sys/") ||
+ normalizedPath.startsWith("/proc/") ||
normalizedPath.startsWith("/dev/") ||
+ normalizedPath.contains(":\\Windows\\") ||
normalizedPath.contains(":\\Program Files\\")) {
+ throw new IllegalArgumentException("Access to system directories is not
allowed");
+ }
+
+ File jarFile;
+ try {
+ // Security: Create File object and immediately get canonical path for
validation
+ jarFile = new File(normalizedPath);
+ String canonicalPath = jarFile.getCanonicalPath();
+
+ // Security: Ensure canonical path doesn't escape intended directory
bounds
+ // This prevents sophisticated path traversal attacks that might bypass
simple string checks
+ if (!canonicalPath.equals(jarFile.getAbsolutePath())) {
+ // Check if the canonical path contains suspicious path traversal
elements
+ // Security: Use the original normalized path for validation instead
of jarFile.getName()
+ String expectedFileName = new File(normalizedPath).getName();
+ if (canonicalPath.contains("..") ||
!canonicalPath.endsWith(expectedFileName)) {
+ throw new IllegalArgumentException(
+ "Invalid JAR file path: canonical path validation failed");
+ }
+ }
+ } catch (java.io.IOException e) {
+ throw new IllegalArgumentException("Invalid JAR file path: " +
e.getMessage());
+ }
+
+ // Security: Ensure the file exists and is a regular file
+ if (!jarFile.exists()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
This path depends on a [user-provided value](2).
[Show more
details](https://github.com/apache/geode/security/code-scanning/130)
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/lifecycle/StartPulseCommand.java:
##########
@@ -71,10 +117,167 @@
}
}
+ /**
+ * Securely browse to a URI with validation to prevent malicious
redirections.
+ *
+ * SECURITY CONSIDERATIONS:
+ *
+ * This method addresses CodeQL vulnerability
java/unvalidated-url-redirection by
+ * validating URLs before passing them to Desktop.browse() to prevent
phishing attacks.
+ *
+ * URL REDIRECTION VULNERABILITIES ADDRESSED:
+ *
+ * 1. UNVALIDATED USER INPUT:
+ * - URL parameter comes directly from user input via @ShellOption
+ * - Could contain malicious URLs pointing to phishing sites
+ * - Direct use in Desktop.browse() enables redirection attacks
+ *
+ * 2. UNTRUSTED MANAGER URLS:
+ * - PulseURL from manager object could be compromised
+ * - May point to malicious sites mimicking legitimate Pulse interface
+ * - Needs validation to ensure safe protocols and hosts
+ *
+ * 3. PHISHING ATTACK PREVENTION:
+ * - Attackers could redirect users to fake login pages
+ * - Could steal credentials or inject malicious content
+ * - URL validation prevents access to non-HTTP/HTTPS protocols
+ *
+ * ENHANCED SECURITY IMPLEMENTATION:
+ *
+ * - DUAL-LAYER VALIDATION: URL string validation before URI creation + URI
validation before
+ * browsing
+ * - Protocol Validation: Only allow HTTP and HTTPS protocols
+ * - Host Validation: Ensure URLs point to expected hosts (localhost or
configured)
+ * - Malicious Protocol Blocking: Reject javascript:, file:, ftp: etc.
+ * - Character Injection Prevention: Block newlines, tabs, and other
dangerous characters
+ * - Pre-URI Validation: Validate URL strings before creating URI objects to
satisfy CodeQL
+ * requirements
+ * - Comprehensive logging for security monitoring
+ *
+ * COMPLIANCE:
Review Comment:
## URL redirection from remote source
Untrusted URL redirection depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/geode/security/code-scanning/129)
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java:
##########
@@ -251,4 +294,126 @@
return result;
}
}
+
+ /**
+ * Security: Validates JAR file paths to prevent path injection attacks.
+ *
+ * This method addresses CodeQL vulnerability java/path-injection by ensuring
+ * that user-provided file paths are safe to access and don't contain
malicious
+ * path traversal sequences.
+ *
+ * SECURITY ENHANCEMENTS:
+ * 1. Pre-validation of path strings before File object creation
+ * 2. Canonical path validation to prevent sophisticated traversal attacks
+ * 3. System directory access prevention (Linux and Windows)
+ * 4. Enhanced path traversal detection with multiple patterns
+ * 5. File type validation and accessibility checks
+ *
+ * COMPLIANCE:
+ * - Fixes CodeQL vulnerability: java/path-injection
+ * - Follows OWASP path traversal prevention guidelines
+ * - Implements defense-in-depth security validation
+ *
+ * @param jarPath The JAR file path to validate
+ * @throws IllegalArgumentException if the path is invalid or unsafe
+ */
+ private void validateJarPath(String jarPath) {
+ if (jarPath == null || jarPath.trim().isEmpty()) {
+ throw new IllegalArgumentException("JAR file path cannot be null or
empty");
+ }
+
+ // Security: Normalize and validate the path string before creating File
object
+ String normalizedPath = jarPath.trim();
+
+ // Security: Prevent path traversal attacks - check for dangerous patterns
+ if (normalizedPath.contains("..") || normalizedPath.contains("~") ||
+ normalizedPath.contains("\\..") || normalizedPath.contains("/..")) {
+ throw new IllegalArgumentException("Invalid JAR file path: path
traversal detected");
+ }
+
+ // Security: Prevent absolute paths to system directories
+ if (normalizedPath.startsWith("/etc/") ||
normalizedPath.startsWith("/sys/") ||
+ normalizedPath.startsWith("/proc/") ||
normalizedPath.startsWith("/dev/") ||
+ normalizedPath.contains(":\\Windows\\") ||
normalizedPath.contains(":\\Program Files\\")) {
+ throw new IllegalArgumentException("Access to system directories is not
allowed");
+ }
+
+ File jarFile;
+ try {
+ // Security: Create File object and immediately get canonical path for
validation
+ jarFile = new File(normalizedPath);
+ String canonicalPath = jarFile.getCanonicalPath();
+
+ // Security: Ensure canonical path doesn't escape intended directory
bounds
+ // This prevents sophisticated path traversal attacks that might bypass
simple string checks
+ if (!canonicalPath.equals(jarFile.getAbsolutePath())) {
+ // Check if the canonical path contains suspicious path traversal
elements
+ // Security: Use the original normalized path for validation instead
of jarFile.getName()
+ String expectedFileName = new File(normalizedPath).getName();
+ if (canonicalPath.contains("..") ||
!canonicalPath.endsWith(expectedFileName)) {
+ throw new IllegalArgumentException(
+ "Invalid JAR file path: canonical path validation failed");
+ }
+ }
+ } catch (java.io.IOException e) {
+ throw new IllegalArgumentException("Invalid JAR file path: " +
e.getMessage());
+ }
+
+ // Security: Ensure the file exists and is a regular file
+ if (!jarFile.exists()) {
+ // Security: Use sanitized filename in error message to prevent
information disclosure
+ String safeFileName = sanitizeFilename(jarFile.getName());
+ throw new IllegalArgumentException("JAR file does not exist: " +
safeFileName);
+ }
+
+ if (!jarFile.isFile()) {
+ // Security: Use sanitized filename in error message to prevent
information disclosure
+ String safeFileName = sanitizeFilename(jarFile.getName());
+ throw new IllegalArgumentException(
+ "Path does not point to a regular file: " + safeFileName);
+ }
+
+ // Security: Validate file extension (basic check for JAR files)
+ // Use sanitized filename for extension validation to prevent path
injection
+ String safeFileName = sanitizeFilename(jarFile.getName());
+ String fileName = safeFileName.toLowerCase();
+ if (!fileName.endsWith(".jar")) {
+ // Security: Use sanitized filename in error message to prevent
information disclosure
+ throw new IllegalArgumentException("File is not a JAR file: " +
safeFileName);
+ }
+
+ // Security: Ensure the file is readable
+ if (!jarFile.canRead()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
This path depends on a [user-provided value](2).
[Show more
details](https://github.com/apache/geode/security/code-scanning/132)
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ImportClusterConfigurationCommand.java:
##########
@@ -173,9 +220,77 @@
}
}
+ /**
+ * Security: Enhanced file upload handling with comprehensive path injection
prevention.
+ *
+ * This method addresses CodeQL vulnerability java/path-injection by
implementing
+ * defense-in-depth validation before creating File objects with
user-controlled paths.
+ *
+ * SECURITY ENHANCEMENTS:
+ * 1. Pre-validation of path strings before File object creation
+ * 2. Canonical path validation to prevent sophisticated traversal attacks
+ * 3. System directory access prevention (Linux and Windows)
+ * 4. Enhanced path traversal detection with multiple patterns
+ * 5. File type validation and accessibility checks
+ * 6. Sanitized error messages to prevent information disclosure
+ *
+ * @return Validated File object safe for processing
+ * @throws IllegalArgumentException if the path is invalid, unsafe, or
inaccessible
+ */
File getUploadedFile() {
List<String> filePathFromShell =
CommandExecutionContext.getFilePathFromShell();
- return new File(filePathFromShell.get(0));
+ String filePath = filePathFromShell.get(0);
+
+ // Security: Comprehensive path validation to prevent path injection
attacks
+ if (filePath == null || filePath.trim().isEmpty()) {
+ throw new IllegalArgumentException("File path cannot be null or empty");
+ }
+
+ // Security: Normalize and validate the path string before creating File
object
+ String normalizedPath = filePath.trim();
+
+ // Security: Prevent path traversal attacks - check for dangerous patterns
+ if (normalizedPath.contains("..") || normalizedPath.contains("~") ||
+ normalizedPath.contains("\\..") || normalizedPath.contains("/..")) {
+ throw new IllegalArgumentException("Invalid file path: path traversal
detected");
+ }
+
+ // Security: Prevent absolute paths to system directories
+ if (normalizedPath.startsWith("/etc/") ||
normalizedPath.startsWith("/sys/") ||
+ normalizedPath.startsWith("/proc/") ||
normalizedPath.startsWith("/dev/") ||
+ normalizedPath.contains(":\\Windows\\") ||
normalizedPath.contains(":\\Program Files\\")) {
+ throw new IllegalArgumentException("Access to system directories is not
allowed");
+ }
+
+ File file;
+ try {
+ // Security: Create File object and immediately get canonical path for
validation
+ file = new File(normalizedPath);
+ String canonicalPath = file.getCanonicalPath();
+
+ // Security: Ensure canonical path doesn't escape intended directory
bounds
+ String expectedFileName = new File(normalizedPath).getName();
+ if (canonicalPath.contains("..") ||
!canonicalPath.endsWith(expectedFileName)) {
+ throw new IllegalArgumentException("Invalid file path: canonical path
validation failed");
+ }
+ } catch (java.io.IOException e) {
+ throw new IllegalArgumentException("Invalid file path: " +
e.getMessage());
+ }
+
+ // Security: Ensure the file exists and is a regular file (not a directory
or special file)
+ if (!file.exists()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
This path depends on a [user-provided value](2).
[Show more
details](https://github.com/apache/geode/security/code-scanning/133)
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ImportClusterConfigurationCommand.java:
##########
@@ -173,9 +220,77 @@
}
}
+ /**
+ * Security: Enhanced file upload handling with comprehensive path injection
prevention.
+ *
+ * This method addresses CodeQL vulnerability java/path-injection by
implementing
+ * defense-in-depth validation before creating File objects with
user-controlled paths.
+ *
+ * SECURITY ENHANCEMENTS:
+ * 1. Pre-validation of path strings before File object creation
+ * 2. Canonical path validation to prevent sophisticated traversal attacks
+ * 3. System directory access prevention (Linux and Windows)
+ * 4. Enhanced path traversal detection with multiple patterns
+ * 5. File type validation and accessibility checks
+ * 6. Sanitized error messages to prevent information disclosure
+ *
+ * @return Validated File object safe for processing
+ * @throws IllegalArgumentException if the path is invalid, unsafe, or
inaccessible
+ */
File getUploadedFile() {
List<String> filePathFromShell =
CommandExecutionContext.getFilePathFromShell();
- return new File(filePathFromShell.get(0));
+ String filePath = filePathFromShell.get(0);
+
+ // Security: Comprehensive path validation to prevent path injection
attacks
+ if (filePath == null || filePath.trim().isEmpty()) {
+ throw new IllegalArgumentException("File path cannot be null or empty");
+ }
+
+ // Security: Normalize and validate the path string before creating File
object
+ String normalizedPath = filePath.trim();
+
+ // Security: Prevent path traversal attacks - check for dangerous patterns
+ if (normalizedPath.contains("..") || normalizedPath.contains("~") ||
+ normalizedPath.contains("\\..") || normalizedPath.contains("/..")) {
+ throw new IllegalArgumentException("Invalid file path: path traversal
detected");
+ }
+
+ // Security: Prevent absolute paths to system directories
+ if (normalizedPath.startsWith("/etc/") ||
normalizedPath.startsWith("/sys/") ||
+ normalizedPath.startsWith("/proc/") ||
normalizedPath.startsWith("/dev/") ||
+ normalizedPath.contains(":\\Windows\\") ||
normalizedPath.contains(":\\Program Files\\")) {
+ throw new IllegalArgumentException("Access to system directories is not
allowed");
+ }
+
+ File file;
+ try {
+ // Security: Create File object and immediately get canonical path for
validation
+ file = new File(normalizedPath);
+ String canonicalPath = file.getCanonicalPath();
+
+ // Security: Ensure canonical path doesn't escape intended directory
bounds
+ String expectedFileName = new File(normalizedPath).getName();
+ if (canonicalPath.contains("..") ||
!canonicalPath.endsWith(expectedFileName)) {
+ throw new IllegalArgumentException("Invalid file path: canonical path
validation failed");
+ }
+ } catch (java.io.IOException e) {
+ throw new IllegalArgumentException("Invalid file path: " +
e.getMessage());
+ }
+
+ // Security: Ensure the file exists and is a regular file (not a directory
or special file)
+ if (!file.exists()) {
+ // Security: Use sanitized filename in error message to prevent
information disclosure
+ String safeFileName = sanitizeFilename(file.getName());
+ throw new IllegalArgumentException("File does not exist: " +
safeFileName);
+ }
+ if (!file.isFile()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
This path depends on a [user-provided value](2).
[Show more
details](https://github.com/apache/geode/security/code-scanning/134)
--
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]
