per_user_logging: username used as a path component without basename()

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-exist is 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.