Comment # 11 on bug 1134132 from
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: