acmesh-official / acmesh-official/acme.sh

Unsafe string escaping throughout main script

Open
#6,463 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.