jdscreations opened a new pull request, #14229:
URL: https://github.com/apache/cloudstack/pull/14229
### Description
`processEncryptionStuff()`'s `encrypt()` builds the `java ... EncryptionCLI`
command by hand-wrapping each dynamic value (jar path, value to encrypt,
management server secret key) in literal double quotes, then hands the
space-joined string to `runCmd()`, which runs it via `subprocess.Popen('
'.join(cmds), shell=True, ...)`.
Double quotes do **not** stop the shell from expanding `"$..."` inside them.
So a password containing `$` — e.g. `pa$sword` — is silently truncated to `pa`
by the shell before it ever reaches `EncryptionCLI`, and the *wrong* value gets
encrypted into `db.properties`. This matches the exact behavior reported in the
issue (decrypting what was stored yields `pa`, not `pa$sword`).
**Fix:** use `shlex.quote()` instead of the manual double-quote wrapping,
for all three dynamic values passed into the command list. `shlex.quote()`
produces shell-safe quoting for arbitrary values, including but not limited to
`$`.
I kept the change scoped to `encrypt()` — the other `runCmd()` caller for
passwords (`mysqlCmds`, ~line 160) already uses single quotes
(`'--password=\'%s\''`), which correctly prevents shell expansion, so that path
isn't affected by this bug and I left it untouched.
Fixes: #14186
### Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [x] Major
### How Has This Been Tested?
The full script is a `.in` template requiring the CloudStack build
(waf/Maven) to substitute `@COMMONLIBDIR@`/`@PYTHONDIR@` and produce a runnable
executable, and `EncryptionCLI` itself lives in `cloudstack-utils.jar`. Rather
than standing up the full build, I isolated the actual bug — shell quoting
through `runCmd()`'s `shell=True` pattern — in a standalone reproduction that
shells out exactly the same way, substituting a trivial stand-in for the Java
program so the arguments it actually receives are directly observable:
```
=== OLD (double-quote wrapping) ===
'pa\npa\n' # password 'pa$sword' arrives truncated to 'pa'
=== NEW (shlex.quote) ===
'pa$sword\npa$sword\n' # password arrives intact
```
Also verified `python3 -m py_compile setup/bindir/cloud-setup-databases.in`
passes (valid Python syntax) and confirmed the same bug is present on this
branch (`4.22`) prior to the fix.
#### How did you try to break this feature and the system with this change?
`shlex.quote()` is the standard-library-correct way to safely pass arbitrary
strings through a `shell=True` invocation — it handles `$`, single quotes,
backticks, semicolons, and whitespace generically, not just the `$` case in the
report. Checked the other three `runCmd()` call sites in this file (mysql
invocation, `build-classpath`, `DatabaseConfig`) to confirm none of them pass
user-controlled dynamic values needing the same treatment, and none were
touched by this change.
--
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]
