acmesh-official / acmesh-official/acme.sh
Unsafe string escaping throughout main script
- Dominant language
- Shell
- Stars
- 47.6k
- Forks
- 5.7k
- Avg merge
- 6d 5h
- Merged PRs (30d)
- 15
Description
A double-quote in a value would allow executable code to be passed to python:
https://github.com/acmesh-official/acme.sh/blob/52e051bb028180a6778391a8310789064ce74127/acme.sh#L1828
Values concatenated in JSON as-is, without escaping. Many are okay because the inputs are known not to contain special characters, but others should be treated as untrusted user-input. Single quoting would aid readability, too, e.g. `_djson='{"status":"deactivated"}'` instead of `_djson="{\"status\":\"deactivated\"}"`.
https://github.com/acmesh-official/acme.sh/blob/52e051bb028180a6778391a8310789064ce74127/acme.sh#L3818
Generating other shell scripts without escaping the content of the variables:
https://github.com/acmesh-official/acme.sh/blob/52e051bb028180a6778391a8310789064ce74127/acme.sh#L6125
The above should be more like below, with `$lesh` having its unnecessary extra quotes removed from its assignment on line 6076, and cron lines need escaping of `%` characters as well:
```sh
lesh=$LE_WORKING_DIR/$PROJECT_ENTRY
...
_c_entry=' --config-home '$(printf '%q' "$_c_home")
...
echo "$random_minute $random_hour * * * $(printf '%q' "$lesh") --cron --home $(printf '%q' "$LE_WORKING_DIR")$_c_entry > /dev/null" | sed 's/%/\\%/g'
```
More of the latter in env file creation, where every item sent to `_setopt` seems to be "escaped" by surrounding in quotes in the call, rather than passing the value as-is, and having that function escape the output correctly:
https://github.com/acmesh-official/acme.sh/blob/52e051bb028180a6778391a8310789064ce74127/acme.sh#L6558
And profile modification:
https://github.com/acmesh-official/acme.sh/blob/52e051bb028180a6778391a8310789064ce74127/acme.sh#L6570
Many instances can be found by search for the literal string `\"`, where the code assumes values will contain no quotes or other escape sequences that are valid in the resultant double-quoted string literal. Some are false positives in informational output.
A function like this would make most less repetitive, the newline deliberately added because `$()` will trim it.:
```sh
_esc () {
printf '%q\n' "$1"
}
```
Used as the `printf` above:
```sh
echo "var=$(_esc "$var")" > some-script
```
I understand it's not an easy fix, as the different environments and programs will need different escaping rules.
Contributor guide
Research direction
Open `acme.sh` and inspect the cited sections (around lines 1828, 3818, 6076-6125, 6558, 6570) where JSON, cron entries, and generated env/profile scripts are built. Start by searching for direct double-quoted interpolations and `_setopt` call sites in those areas to map all risky write points. No test file is named in the issue; completion is done when those script-generation paths consistently escape user-derived values and the produced cron/env/profile output no longer risks shell injection.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- shell
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100