Security issues were found by SUSE Security team

[hillwoodroc]

https://bugzilla.opensuse.org/show_bug.cgi?id=1130388

So I reviewed deepin-clone and there are a couple of issues regarding
security. They need to be fixed before we can accept the package:

1. in GUI mode deepin-clone creates "/tmp/.deepin-clone.log" and follows
   symlinks there. This file needs to be open()ed with
   O_NOFOLLOW|O_CREAT|O_EXCL. Or even better using a non predictable temporary
   file in /tmp using QTemporaryFile.

2. temporaryMountDevice() uses a fixed path
   /tmp/.deepin-clone/mount/ to temporarily mount a file
   system there. These paths can be prepared by an attacker and symlinks will
   be followed during mounting. If the attacker quickly enters the mount then
   it probably can also prevent the following unmount. This logic can e.g. be
   triggered by running deepin-clone -i /dev/sdX.

An attacker can thus cause the file system to be permanently mounted at an
arbitrary location in the file system.

3. Helper::getPartitionSizeInfo() uses /tmp/partclone.log as a fixed path
   during execution of partclone. The same issues about symlink attacks etc.
   like in 1) apply here. This needs to be a non-predictable path, or a path in
   a directory not accessible to regular users.

4. similarly in BootDoctor::fix() the fixed path /tmp/repo.iso is created and
   the fixed directory /tmp/.deepin-clone is used. The same concerns as in 1)
   and 3) apply.

5. /tmp/.deepin-clone.log and /var/log/deepin-clone.log are both world
   readable. Log files whould not be world readable when they come from a
   program running as root as it may leak information valuable to an attacker.

6. in helper.cpp:986:

if (QFile::exists(QString("/proc/%1").arg(process->pid()))) {
    process->terminate();
    process->waitForFinished();
} else {

I don't know if this check is really necessary but if it is then it is a
race condition and could be used to kill an unrelated process in case the
child PID is replaced by some other process unrelated to deepin-clone.

7. in GUI and pkexec mode deepin-clone opens the PKEXEC_UIDs
   ~/.pam_environment while following symlinks and not checking file types.
   While it can be argued that the executor that successfully was granted
   access by pkexec shouldn't be an attacker it would still be a better style
   not to open this file as root. It should be opened only with
   setfsuid(PKEXEC_UID).

Non security issues but style/robustness issues:

8. isBlockSpecialFile() returns true if the path starts with /dev? That sounds
   like a hack and could lead to surprises!
9. DDiskInfo::getInfo() under some circumstances also creates the file? Sounds
   like an unexpected side effect!

The issue 1) and 2) and probably also 3) and 4) could require CVE assignments
since these are realistic security issues that can be exploited by local
regular users in the system.

[zccrs]

#18

[justforlxz]

Sorry, this issue will be closed soon. If it is necessary to discuss it again, please create a new issue.