per_user_logging: username used as a path component without basename()
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
Research direction
Start in program/lib/Roundcube/rcube.php at get_user_log_dir(), and trace how it is reached when per_user_logging is enabled. Verify that usernames containing traversal components can no longer place logs outside the configured log_dir, while valid usernames still resolve to writable per-user directories; check existing PHP tests for the appropriate coverage location.
Written by the indexing model from the issue text.
Description
Summary
get_user_log_dir() concatenates the authenticated username into a filesystem path without basename() or any traversal filtering. A username containing ../ resolves outside the configured log_dir.
Affected code
program/lib/Roundcube/rcube.php:
protected function get_user_log_dir()
{
$log_dir = $this->config->get('log_dir', RCUBE_INSTALL_PATH . 'logs');
$user_name = $this->get_user_name();
$user_log_dir = $log_dir . '/' . $user_name;
return !empty($user_name) && is_writable($user_log_dir) ? $user_log_dir : false;
}
Only reachable when $config['per_user_logging'] is enabled, which is not the default.
Evidence
Measured on current master, PHP 8.5.10, inside a private probe tree:
sanitised with basename(): NO
gated by is_writable(): YES
username resulting path accepted?
alice <root>/logs/alice no
../elsewhere <root>/logs/../elsewhere YES -> <root>/elsewhere
../does-not-exist <root>/logs/../does-not-exist no
../../../../tmp <root>/logs/../../../../tmp no
bob/../../elsewhere <root>/logs/bob/../../elsewhere no
Impact — deliberately stated narrowly
I want to be precise about how limited this is, because the is_writable() check does most of the work:
- The traversal target must already exist and be writable by the web server user.
../does-not-existis rejected; no directory is created. - The only case that succeeded in my probe was
../elsewhere, where I had created that directory beforehand. - It requires a username containing
../to be provisioned in the IMAP or directory backend, which is outside Roundcube's control.
So this is not an arbitrary-file-write primitive. What it is: a missing input constraint that lets log output land outside the intended directory when both preconditions happen to line up. I'd classify it as hardening rather than a vulnerability, which is why I'm filing it publicly rather than reporting it privately.
Suggested fix
Constrain the path component:
$user_name = basename($this->get_user_name());
or, stricter, reject usernames that are not a safe filename before using them as a path component. basename() alone already neutralises every case in the table above.
Happy to send a PR.
- Dominant language
- PHP
- Stars
- 7.2k
- Forks
- 1.8k
- Avg merge
- 5d 12h
- Merged PRs (30d)
- 2
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from roundcube/roundcubemail
-
Difficulty 1/5 Under an hour Newbie friendliness 92/100
roundcube/roundcubemail#10339 · 3 comments ·
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
roundcube/roundcubemail#10337 · 1 comment ·
-
Difficulty 1/5 Under an hour Newbie friendliness 92/100
roundcube/roundcubemail#10336 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
roundcube/roundcubemail#10328 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
roundcube/roundcubemail#10326 ·
All issues in roundcube/roundcubemail
Similar issues
-
priority: p3
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
googleapis/librarian#7636 ·
-
0. Needs triage bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
nextcloud/fulltextsearch#1011 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
phpstan/phpstan-doctrine#794 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
Automattic/static-site-importer#1767 ·