Do you have any schedule for security issues on file manager?

SUMMARY | 问题概要

Forward from https://bugzilla.opensuse.org/show_bug.cgi?id=1134132 and https://bugzilla.opensuse.org/show_bug.cgi?id=1134131

Basically we can merge this bug here with the other bug 1134131. It is about the same basic software and mechanisms.

So the D-Bus interface in this package changed quite a bit. There is also some authentication in there for some D-Bus methods now. However the authentication is buggy:

1) PolicyKitHelper::checkAuthorization uses the UnixProcessSubject which is racy. PIDs can change at any time. The SytemBusName polkit subject needs to be used to make this proper.

2) Similarly in AccessControlManager the methods SetAccessPolicy and SetVaultAccessPolicy uses the callers PID to check against a whitelist of process names. This is also racy, the caller can replace its own process by a different one that matches the expectations of this check.

RevocationManager: it is unclear what this is used for, but anybody can call pushEvent, popEvent without authentication. This can DoS memory but also cause something else, whatever popEvent() means for the deepin environment.

TagManagerDaemon::disposeClientData: this method changes a sqlite database and is unauthenticated. Doesn't look trustworthy.

VaultManager interface:

sysUserChanged is unauthenticated, causes a heap allocation.
checkAuthentication method has an unclear use (why expose an authentication method on D-Bus?) but it also uses the UnixProcessSubject, so it's racy
all other methods are also unauthenticated

VaultManager2 interface:

VaultBruteForcePrevention::isValidInvoker() always returns true, thus all methods are unauthenticated

UserShareManager interface:

UnmountSambaShare tries to determine whether the target path is controlled by the caller's UID. This method allows to accesses arbitrary paths, also symlinks, which is racy. For example passing ../../../../home/myusername allows to escape the destined directory. Also the share name can contain '--' as prefix and would thus in theory allow to pass arbitrary arguments to the smbcontrol invocation:
```
QString cmd = QString("smbcontrol smbd close-share %1").arg(sharename);
qDebug() << "cmd==========" << cmd;
p.start(cmd);
```
I'm not completely sure how QProcess::start() behaves here, but it could even be possible to inject multiple parameters or shell syntax here.

So all in all things have not really improved a lot here. This still cannot be accepted.

STEPS TO REPRODUCE | 复现步骤

None

OBSERVED RESULT | 观察到的结果

No response

EXPECTED RESULT | 期望的结果

No response

SOFTWARE/OS VERSIONS | 软件/系统版本信息

All

ADDITIONAL INFORMATION | 额外补充

No response

linuxdeepin / developer-center