/usr/sbin/userdel_local.sh and /usr/bin/user-managerd --removeUserFiles $1
TL;DR: VERDICT: likely harmless, but the scripts must be looked into.
jolla-usermanagerd, through its script /usr/sbin/userdel_local.sh calls /usr/bin/user-managerd --removeUserFiles $1
which does this:
QDir dir(USER_ENVIRONMENT_DIR.arg(uid));
if (dir.removeRecursively())
Where USER_ENVIRONMENT_DIR is defined a bit higher in the file as:
const auto USER_ENVIRONMENT_DIR = QStringLiteral("/home/.system/var/lib/environment/%1");
So, IF uid is defined (e.g. as 999), dir will be /home/.system/var/lib/environment/9999 BUT if uid null or empty, dir will be /home/.system/var/lib/environment/, and the call will remove ALL subdirs from ALL users.
AFAICS, uid is not checked for null or empty.
However, even if there were a bug here, the effects would be small, as /home/.system/var/lib/environment/ only contains some files setting user environment, like the selected locale options.
Still, in that same function, after we did dir.removeRecursively(), we proceed to execute the scripts from executeScripts(uid, USER_REMOVE_SCRIPT_DIR);, where USER_REMOVE_SCRIPT_DIR is the one mentioned above:
const auto USER_REMOVE_SCRIPT_DIR = QStringLiteral("/usr/share/user-managerd/remove.d");
user-managerd::removeUser
TL;DR: VERDICT: inconclusive
So lets look at user-managerd::removeUser:
{
if (!checkAccessRights(uid))
return;
if (uid == OWNER_USER_UID) {
auto message = QStringLiteral("Can not remove device owner");
qCWarning(lcSUM) << message;
sendErrorReply(QDBusError::InvalidArgs, message);
return;
}
if (uid == currentUser()) {
auto message = QStringLiteral("Can not remove current user");
qCWarning(lcSUM) << message;
sendErrorReply(QDBusError::InvalidArgs, message);
return;
}
m_exitTimer->start();
if (uid != SAILFISH_USERMANAGER_GUEST_UID && !removeHome(uid)) {
qCWarning(lcSUM) << "Removing user home failed";
}
removeUserFiles(uid);
if (!m_lu->removeUser(uid)) {
auto message = QStringLiteral("User remove failed");
qCWarning(lcSUM) << message;
sendErrorReply(QStringLiteral(SailfishUserManagerErrorUserRemoveFailed), message);
} else {
emit userRemoved(uid);
}
Interesting, there is a check for OWNER - so this is meant to prevent exactly what we see in the bug report.
Does this work right: if (uid == currentUser()) ?
Hm:
uint SailfishUserManager::currentUser()
{
m_exitTimer->start();
uid_t uid;
if (sd_seat_get_active("seat0", nullptr, &uid) < 0) {
auto message = QStringLiteral("Failed to get current user id");
qCWarning(lcSUM) << message;
sendErrorReply(QStringLiteral(SailfishUserManagerErrorGetUidFailed), message);
return SAILFISH_UNDEFINED_UID;
}
return uid;
}
Assuming systemd can actually correctly find out the uid from seat0 through sd_seat_get_active (big assumption here!), we MIGHT get back SAILFISH_UNDEFINED_UID. AFAICS that is used in various sanity checking functions, including checkAccessRights.
I could not find where SAILFISH_UNDEFINED_UID is actually defined - does this mean it is indeed “undefined” everywhere? (Similarly there is no definition for OWNER_USER_ID. These must be set in the build environment beforehand? No idea.)
But assuming those checks are all sound, what’s this?
if (uid != SAILFISH_USERMANAGER_GUEST_UID && !removeHome(uid)) {
bool SailfishUserManager::removeHome(uint uid)
{
QString home = m_lu->homeDir(uid);
if (home.isEmpty())
return false;
return removeDir(home);
}
bool SailfishUserManager::removeDir(const QString &dir)
{
QDir directory(dir);
if (!directory.removeRecursively()) {
qCWarning(lcSUM) << "Removing directory failed";
return false;
}
return true;
}
Okay, here we actually delete stuff so this looks dangerous.
AFAICS it is also the only place where user homes are deleted so far.
What’s m_lu, which resolves the actual dir to remove? It’s an instance of libuser, via m_lu(new LibUserHelper()).We find homeDir in src/libuserhelper.cpp which in turn uses libuser. lu_user_lookup_id and lu_ent_get_first_string_in are used to retrieve the home directory from passwd.
