Skip to content

fix(build): quote paths in root logrotate cleanup to prevent arbitrary deletion#1602

Open
pragmaxim wants to merge 1 commit into
masterfrom
fix/logrotate-shell-injection
Open

fix(build): quote paths in root logrotate cleanup to prevent arbitrary deletion#1602
pragmaxim wants to merge 1 commit into
masterfrom
fix/logrotate-shell-injection

Conversation

@pragmaxim

Copy link
Copy Markdown
Contributor

Summary

The generated Blockbook Debian package installs a daily cron wrapper that runs bin/logrotate.sh as root, while postinst chowns the per-coin logs/ directory to the unprivileged blockbook-<coin> service user. The cleanup script expanded the discovered log path unquoted in both fuser and rm -f $log:

find $LOGS -mtime +7 -type f -print0 | while read -r -d $'\0' log; do
    if ! fuser -s $log; then
        rm -f $log
    fi
done

A file created by the service user whose name contains shell word-splitting tokens — e.g. blockbook-bitcoin.log -rf /opt/coins/data/bitcoin/backend — is split into multiple rm operands. On GNU rm, -rf is honored even after a leading operand, so the root cron job can be steered into recursively deleting an attacker-chosen path outside the log tree (backend node data, Blockbook RocksDB, config, other coins' data). The find -mtime +7 filter is not a barrier since the attacker controls the file's mtime.

This is a local privilege-boundary issue (root-mediated arbitrary recursive deletion), reachable by anyone who can write as the Blockbook service user — the realistic outcome of a Blockbook process compromise.

Fix

find "$LOGS" -mtime +7 -type f -print0 | while read -r -d $'\0' log; do
    if ! fuser -s -- "$log"; then
        rm -f -- "$log"
    fi
done

Quoting keeps the path a single word; -- terminates option parsing so a name beginning with - can't be interpreted as a flag.

Scope

Change is limited to build/templates/blockbook/logrotate.sh. postinst's unquoted $dir expansions use build-time template values (BlockbookInstallPath, Coin.Alias), not runtime attacker input, so they carry no injection vector and are left untouched.

As defense-in-depth follow-up, package log cleanup could move to a standard logrotate.d config so a root process never walks a service-user-owned tree treating paths as shell words.

🤖 Generated with Claude Code

…y deletion

The generated Debian package installs a daily cron wrapper that runs
bin/logrotate.sh as root, while postinst chowns the per-coin log
directory to the unprivileged Blockbook service user. The cleanup
script expanded the discovered log path unquoted in both `fuser` and
`rm -f $log`, so a file whose name contains shell word-splitting tokens
(e.g. "blockbook-bitcoin.log -rf /opt/coins/data/bitcoin/backend")
would be split into multiple operands. On GNU rm, `-rf` is honored even
after an earlier operand, letting the service user cause the root cron
job to recursively delete an attacker-chosen path outside the log tree
(backend/RocksDB data, config, other coins' data).

Quote "$LOGS" and "$log" and pass `--` before operands so an
attacker-controlled path is always treated as a single non-option
argument.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pragmaxim pragmaxim requested a review from cranycrane July 3, 2026 08:24
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