Skip to content

fix(manager): run pg backups without a shell - #2451

Open
elcreator wants to merge 1 commit into
evolution-cms:3.5.xfrom
elcreator:fix/bkmanager-pgsql-shell
Open

fix(manager): run pg backups without a shell#2451
elcreator wants to merge 1 commit into
evolution-cms:3.5.xfrom
elcreator:fix/bkmanager-pgsql-shell

Conversation

@elcreator

Copy link
Copy Markdown

The PostgreSQL branches of the backup manager built psql and pg_dump command lines by concatenation and handed them to exec(). The snapshot name from the restore form and the table names from the checkbox list went in unquoted, so bk_manager - a permission whose intended reach is arbitrary SQL through the restore form, not arbitrary shell - was enough to append a command of its own. The password rode in argv, where ps and /proc//cmdline show it to every local account.

All three branches now go through DatabaseBackupService, which invokes the client with an argument list and the password in the environment. No shell parses either, so an argument holding a semicolon stays one argument and nothing depends on escaping being right. This is the same form createSnapshot() already used, which also means these paths start working on Windows, where a leading PGPASSWORD= assignment was a cmd.exe syntax error.

The dump no longer lands in assets/backup/temp.php. That path is inside the web root and the default ht.access excludes assets/ from every rule it has, so a full dump sat there readable by anyone who guessed the name; it now goes to the snapshot directory under a name that is not guessable. The deny rule written into that directory gained the Apache 2.4 form - Order/Deny alone is 2.2 syntax and 2.4 without mod_access_compat answers 500, leaving the directory served.

The snapshot to restore is resolved against the snapshot directory and rejected if it lands outside, which also closes the traversal on the MySQL and SQLite branches, where the name reached import_sql_from_file() unchecked. The drop-tables checkbox is honoured again; it built an argument the command never used.

The PostgreSQL branches of the backup manager built psql and pg_dump
command lines by concatenation and handed them to exec(). The snapshot
name from the restore form and the table names from the checkbox list
went in unquoted, so bk_manager - a permission whose intended reach is
arbitrary SQL through the restore form, not arbitrary shell - was enough
to append a command of its own. The password rode in argv, where ps and
/proc/<pid>/cmdline show it to every local account.

All three branches now go through DatabaseBackupService, which invokes
the client with an argument list and the password in the environment.
No shell parses either, so an argument holding a semicolon stays one
argument and nothing depends on escaping being right. This is the same
form createSnapshot() already used, which also means these paths start
working on Windows, where a leading PGPASSWORD= assignment was a cmd.exe
syntax error.

The dump no longer lands in assets/backup/temp.php. That path is inside
the web root and the default ht.access excludes assets/ from every rule
it has, so a full dump sat there readable by anyone who guessed the
name; it now goes to the snapshot directory under a name that is not
guessable. The deny rule written into that directory gained the Apache
2.4 form - Order/Deny alone is 2.2 syntax and 2.4 without
mod_access_compat answers 500, leaving the directory served.

The snapshot to restore is resolved against the snapshot directory and
rejected if it lands outside, which also closes the traversal on the
MySQL and SQLite branches, where the name reached import_sql_from_file()
unchecked. The drop-tables checkbox is honoured again; it built an
argument the command never used.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YSZQgDv5ASxQaiYd5C1nJR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant