fix(user): confine the home folder naming rule to the data directory

[DeepDiver1975]

Summary

The LDAP "User Home Folder Naming Rule" setting can name a per user home from an
LDAP attribute (attr:homeDirectory). That value was accepted essentially
verbatim, so an LDAP-controlled home pointing at the ownCloud code directory gave
the user read and write access to the application's own PHP files — remote code
execution. This PR confines the home to the data directory.

Reported internally as OC10-133.

Root cause

UserEntry::getHome() (lib/User/UserEntry.php) validated the attribute value
with nothing more than a check for a leading slash:

if ($path[0] !== '/') {
    $path = $dataDir . '/' . $path;
}
// ... returned as-is

Two ways through:

• Absolute — /var/www/owncloud/apps passes the [0] !== '/' check untouched
  and becomes the user's home.
• Relative — concatenated onto the data directory with no normalization, so
  ../../apps escapes it.

The value is then persisted on the account and used to construct the user's
Storage\Home. Local::getSourcePath() looks like it would catch this, but it
anchors its containment check to a realDataDir derived from the home it is
validating — it effectively validates the path against itself.

Note a files/ offset applies (Root::getUserFolder()), so the code root is not
listed directly; RCE still lands via the files/ directory that gets created
inside the target, or by aiming at /var/www/owncloud/apps where writing a new
app directory is enough.

Fix

Reject a home that does not resolve inside the configured datadirectory.
Installations that legitimately keep homes elsewhere (a separate NFS mount, say)
list the permitted base directories in the new user_ldap.home_base_dirs
config option. The refusal logs both the offending path and the option name.

Two details worth calling out:

• Symlinks are resolved on the longest leading part of the path that exists on
  disk. realpath() alone will not do — a home is created lazily on first login,
  so it usually does not exist yet at validation time. Resolving the existing
  prefix keeps a symlinked data directory working (a legitimate setup) while
  stopping a symlink inside the data directory from being used to escape.
• Prefix siblings are rejected: the comparison includes the trailing
  separator, so /data-evil is not treated as contained in /data.

Testing

New tests in tests/unit/User/UserEntryTest.php:

• testGetHomeOutsideDataDirIsRefused — 7 cases (app root, apps dir, /etc,
  traversal, prefix sibling, …)
• testGetHomeEscapingViaSymlinkIsRefused — real symlink on disk
• testGetHomeUnderSymlinkedDataDirIsAccepted — the legitimate setup keeps working
• testGetHomeOutsideDataDirAllowedByConfig — the opt-out is honoured

Two pre-existing tests (testGetHomeAttributeWithAbsolutePath /
…WithRelativePath) needed their getSystemValue mocks relaxed, since the fix
reads a second config key.

Full app unit suite green: 259 tests, 544 assertions, OK.
php-cs-fixer and php -l clean on both changed files.

⚠️ Breaking change

An installation whose LDAP homes legitimately resolve outside datadirectory
will refuse those logins after upgrading until user_ldap.home_base_dirs (or
core's user.home_base_dirs) lists the base directory. The error naming the path
and the option is written to the log, so affected installs are diagnosable.

Known limitation (follow-up)

Accounts provisioned before this fix keep the already-persisted home in the
accounts table: core's syncHome() only sets a home when none is stored, so
such accounts stay exploitable until the row is corrected. Deliberately out of
scope here; tracked as OC10-139, which will add a read-only occ report for
accounts whose stored home falls outside the permitted base directories.

Related

• Documentation: docs(admin_manual): document user home containment and user.home_base_dirs docs-server#1573
• Core counterpart (defense in depth for all backends providing a home):
  fix(user): confine backend provided user homes to the data directory core#41752

🤖 Generated with Claude Code

[jvillafanez]

Although the $path won't be empty in this case, it seems better to either ensure it won't happen here, or document that is the caller's responsibility to ensure that the $path isn't empty.

[DeepDiver1975]

Good instinct — following it up turned out to find a real bug, not just a missing doc line.

You're right that $path (the home) can't be empty here: it is guarded absolute before normalizePath() is reached. But the other argument was not. getHome() skipped an empty base dir like this:

if (!\is_string($baseDir) || $baseDir === '') {
    continue;
}
if (self::isContainedIn($path, self::normalizePath($baseDir))) {

The guard ran before normalization — and ., ./ and ./. normalize to the empty string, so they slipped past it. isContainedIn() then computed:

\strpos($path, \rtrim('', '/') . '/') === 0   //  strpos($path, '/') === 0

which is true for every absolute path. A single stray '.' in user_ldap.home_base_dirs silently disabled the containment check completely.

It's also worth noting that an empty value is asymmetric — which is exactly why your comment was worth chasing:

isContainedIn('',     '/var/www/owncloud/data') = false   <- fail-safe
isContainedIn('/etc', '')                       = true    <- fail-OPEN

Harmless in the first argument, disabling in the second.

Fixed in 1f7034b by requiring the base dir to be an absolute path before normalizing it. One condition covers '', '.', './', './.', '..' and non-strings, and it additionally rules out a relative entry such as 'data' — realpath() would have resolved that against the working directory of whichever process ran the check, so the answer would differ between the web server and occ.

Then both halves of what you asked for:

• Ensured it won't happen: isContainedIn() now returns false for an empty $baseDir in its own right, so the predicate can't fail open even if a future caller forgets to guard.
• Documented the responsibility: both normalizePath() docblocks now state that passing an absolute path is the caller's job, and why.

Verified with a negative control — reverting only the guard reproduces exactly the three '.'-shaped failures and nothing else; restoring it turns them green. owncloud/core#41752 had the identical defect in SyncService::verifyHomeLocation() and is fixed the same way.

New tests: testGetHomeIsRefusedForUnusableBaseDir (8 cases).

[jvillafanez]

I miss some tests for /var/www/../../opt or similar (absolute path with .. in the middle).
Also something like /var/www/./owncloud

[DeepDiver1975]

Added in 1f7034b. To be straight about the result: I checked your exact inputs against the real helpers before writing the tests, and they already resolved correctly — normalizePath() resolves dot segments before comparing, so /var/www/../../opt → /opt and /var/www/./owncloud → /var/www/owncloud, both refused. So this is coverage for behaviour that was already right, not a bug fix. Still worth having — nothing was pinning it.

Five cases added to providesHomeOutsideDataDir:

'absolute traversal mid-path' => ['/var/www/../../opt'],
'absolute traversal out of data dir' => ['/var/www/owncloud/data/../../apps'],
'absolute traversal with dot segments' => ['/var/www/./owncloud/./apps'],
'absolute traversal to root' => ['/..'],
'absolute dot segments to code root' => ['/var/www/owncloud/data/../.'],

I also added a companion provider for the other direction (testGetHomeWithDotSegmentsInsideDataDirIsAccepted), so the suite isn't only proving that things get rejected — a check that rejected everything would have passed the old provider. It asserts that dot segments landing back inside the data directory are still accepted and normalize as expected: /var/www/owncloud/data/./alice, .../foo/../alice, //data//alice, a trailing slash, the data directory itself, and the relative ./alice.

UserEntryTest is now 76 tests / 125 assertions, green.

Chasing your other comment did turn up a real fail-open defect — see the reply on UserEntry.php.