https://bugzilla.suse.com/show_bug.cgi?id=1134132 https://bugzilla.suse.com/show_bug.cgi?id=1134132#c11 --- Comment #11 from Matthias Gerstner <matthias.gerstner@suse.com> --- 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. -- You are receiving this mail because: You are on the CC list for the bug.