YLChen-007 opened a new issue, #12005:
URL: https://github.com/apache/cloudstack/issues/12005

   ## **Description:**
   
   CLOUDSTACK VERSION
   [4.22.0.0](https://github.com/apache/cloudstack/releases/tag/4.22.0.0)
   
   ### Summary
   The `Script.java` class provides a command execution framework with password 
masking capabilities, but the masking logic is critically flawed. It only 
obscures parameters following specific flags (`-y`/`-z`) or certain URI 
patterns, leaving credentials passed through direct parameter addition (e.g., 
`script.add(username, newPassword)`) completely exposed. This results in 
plaintext password logging at both DEBUG and WARN levels throughout the command 
execution lifecycle.
   
   ### Severity
   **Critical** - Systematic password exposure across multiple log levels 
(DEBUG and WARN) affects all components using the Script framework. WARN-level 
logging is typically enabled in production, making this vulnerability active in 
live deployments.
   
   ---
   
   ## **Vulnerability Details**
   
   ---
   
   ## **Root Cause Analysis**
   
   ### 1. **Inadequate Password Masking Logic**
   
   The `cloud.utils.script.Script#buildCommandLine()` method only masks 
parameters in very specific scenarios:
   
   ```java
   protected String buildCommandLine(String[] command) {
       StringBuilder builder = new StringBuilder();
       boolean obscureParam = false;
       for (int i = 0; i < command.length; i++) {
           String cmd = command[i];
           
           // Only handles specific URI patterns
           if (sanitizeViCmdParameter(cmd, builder) || 
               sanitizeRbdFileFormatCmdParameter(cmd, builder)) {
               continue;
           }
           
           if (obscureParam) {
               builder.append("******").append(" ");  // ✅ Masks ONLY if flag 
detected
               obscureParam = false;
           } else {
               builder.append(cmd).append(" ");  // ❌ All other params logged 
plaintext
           }
           
           // ❌ ONLY triggers masking for -y/-z flags
           if ("-y".equals(cmd) || "-z".equals(cmd)) {
               obscureParam = true;
               _passwordCommand = true;
           }
       }
       return builder.toString();
   }
   ```
   
   **Critical Flaw**: The masking logic assumes passwords are always preceded 
by `-y` or `-z` flags. Any other parameter passing pattern bypasses masking 
entirely.
   
   ---
   
   ### 2. **Direct Parameter Addition Bypasses Masking**
   
   The `CitrixUpdateHostPasswordCommandWrapper` uses direct parameter addition:
   
   ```java
   // CitrixUpdateHostPasswordCommandWrapper.java (lines 34-45)
   final Script script = new Script(scriptPath + scriptName, _timeout, logger);
   script.add(username, newPassword);  // ❌ No flag, no masking
   final String result = script.execute();
   ```
   
   The `add()` method simply appends parameters to the command array:
   
   ```java
   public void add(String... params) {
       for (String param : params) {
           _command.add(param);  // ❌ Direct addition, no masking metadata
       }
   }
   ```
   
   **Result**: When `buildCommandLine()` processes these parameters, they don't 
follow `-y`/`-z` flags, so they're logged in plaintext.
   
   ---
   
   ### 3. **Multiple Logging Points Expose Credentials in 
com.cloud.utils.script.Script#execute**
   
   #### ** Point 1: DEBUG Level Logging **
   
   ```java
   public String execute(OutputInterpreter interpreter) {
       String[] command = _command.toArray(new String[_command.size()]);
       String commandLine = buildCommandLine(command);
       
       if (_logger.isDebugEnabled() && !avoidLoggingCommand) {
           // ❌ Logs full command with plaintext password
           _logger.debug(String.format("Executing command [%s].", 
                         commandLine.split(KeyStoreUtils.KS_FILENAME)[0]));
       }
       // ...
   }
   ```
   
   **Exposure**: Every command execution logs the full command line at DEBUG 
level unless `avoidLoggingCommand` is explicitly set.
   
   ---
   
   #### ** Point 2: WARN Level Logging on Timeout **
   
   ```java
   TimedOutLogger log = new TimedOutLogger(_process);
   Task timedoutTask = new Task(log, ir);
   
   _logger.trace(String.format("Running timed out task of process [%s] for 
command [%s].", 
                 processPid, commandLine));
   
   if (!_passwordCommand) {
       // ❌ Logs full command with password at WARN level
       _logger.warn(String.format("Process [%s] for command [%s] timed out. 
Output is [%s].", 
                    processPid, commandLine, timedoutTask.getResult()));
   } else {
       _logger.warn(String.format("Process [%s] for command [%s] timed out.", 
                    processPid, commandLine));
   }
   ```
   
   **Critical Issue**: The `_passwordCommand` flag is only set to `true` when 
`-y`/`-z` flags are detected. Since `CitrixUpdateHostPasswordCommandWrapper` 
doesn't use these flags:
   - `_passwordCommand` remains `false`
   - Timeout warnings log the **full command with plaintext password at WARN 
level**
   - **WARN logs are typically enabled in production**
   
   ---
   
   #### ** Point 3: WARN Level Logging on Execution Failure **
   
   ```java
   _logger.debug(String.format("Exit value of process [%s] for command [%s] is 
[%s].", 
                 processPid, commandLine, _process.exitValue()));
   
   BufferedReader reader = new BufferedReader(new 
InputStreamReader(_process.getInputStream()), 128);
   
   String error;
   if (interpreter != null) {
       error = interpreter.processError(reader);
   } else {
       error = String.valueOf(_process.exitValue());
   }
   
   // ❌ Logs full command with password at WARN level on failure
   _logger.warn(String.format("Process [%s] for command [%s] encountered the 
error: [%s].", 
                processPid, commandLine, error));
   ```
   
   **Exposure**: Any script execution failure logs the full command at WARN 
level, including plaintext passwords.
   
   ---
   
   #### ** Point 4: WARN Level Logging on Exception **
   
   ```java
   } catch (SecurityException ex) {
       // ❌ Logs command with password at WARN level
       _logger.warn(String.format("Exception [%s] occurred. This may be due to 
an attempt of " +
                    "executing command [%s] as non root.", ex.getMessage(), 
commandLine), ex);
       return stackTraceAsString(ex);
   } catch (Exception ex) {
       // ❌ Logs command with password at WARN level
       _logger.warn(String.format("Exception [%s] occurred when attempting to 
run command [%s].", 
                    ex.getMessage(), commandLine), ex);
       return stackTraceAsString(ex);
   }
   ```
   
   **Exposure**: Any exception during script execution logs the full command at 
WARN level.
   
   ---
   
   ### 4. **Missing `avoidLoggingCommand` Flag**
   
   The `Script` class provides an `avoidLoggingCommand` flag to suppress 
logging:
   
   ```java
   // Script.java
   private boolean avoidLoggingCommand = false;
   
   public void setAvoidLoggingCommand(boolean avoidLoggingCommand) {
       this.avoidLoggingCommand = avoidLoggingCommand;
   }
   ```
   
   **Problem**: `CitrixUpdateHostPasswordCommandWrapper` never calls 
`setAvoidLoggingCommand(true)`:
   
   ```java
   // CitrixUpdateHostPasswordCommandWrapper.java
   final Script script = new Script(scriptPath + scriptName, _timeout, logger);
   script.add(username, newPassword);
   // ❌ Missing: script.setAvoidLoggingCommand(true);
   final String result = script.execute();
   ```
   
   **Result**: Even the DEBUG-level protection is not applied.
   
   ---
   
   ## **Complete Vulnerability Chain**
   
   ```
   1. CitrixUpdateHostPasswordCommandWrapper creates Script instance
      ↓
   2. Calls script.add(username, newPassword) without -y/-z flags
      ↓
   3. Does NOT call script.setAvoidLoggingCommand(true)
      ↓
   4. Script.execute() calls buildCommandLine()
      ↓
   5. buildCommandLine() doesn't detect -y/-z flags
      ↓ 
   6. Password NOT masked in command string
      ↓
   7. _passwordCommand remains false
      ↓
   8. Password logged at DEBUG level
      ↓
   9. On timeout/failure/exception: Password logged at WARN level 
   ```
   
   ---
   
   ## **Exposed Sensitive Data**
   
   | Data Type | Exposure Point | Log Level | Production Impact |
   |-----------|----------------|-----------|-------------------|
   | New host password | DEBUG logging | DEBUG | If DEBUG enabled |
   | New host password | Timeout logging | **WARN** | **Always in production** |
   | New host password | Failure logging | **WARN** | **Always in production** |
   | New host password | Exception logging | **WARN** | **Always in 
production** |
   


-- 
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]

Reply via email to