CVE-2019-13226: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: deepin-clone before 1.1.3 uses a predictable path /tmp/.deepin-clone/mount/<block-dev-basename> in the Helper::temporaryMountDevice() function to temporarily mount a file system as root. An unprivileged user can prepare a symlink at this location to have the file system mounted in an arbitrary location. By winning a race condition, the attacker can also enter the mount point, thereby preventing a subsequent unmount of the file system.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
CVSS v3 base score: 7
Published: 2019-07-04T12:15Z
Repository: https://github.com/linuxdeepin/deepin-clone
Language: C++
Files changed: app/src/corelib/ddevicediskinfo.cpp, app/src/corelib/helper.cpp, app/src/corelib/helper.h, app/src/fixboot/bootdoctor.cpp, app/src/main.cpp
Commit date: 2019-06-01 12:50:37 +0800
Commit message: fix: Do not use the "/tmp" directory

https://github.com/linuxdeepin/deepin-clone/issues/16
https://bugzilla.opensuse.org/show_bug.cgi?id=1130388

Diff: diff --git a/app/src/corelib/ddevicediskinfo.cpp b/app/src/corelib/ddevicediskinfo.cpp
index d5e0e77..48a54ba 100644
--- a/app/src/corelib/ddevicediskinfo.cpp
+++ b/app/src/corelib/ddevicediskinfo.cpp
@@ -301,13 +301,13 @@ bool DDeviceDiskInfoPrivate::openDataStream(int index)
             }
         }
 
         if (currentMode == DDiskInfo::Read) {
             const QString &executer = Helper::getPartcloneExecuter(part);
-            process->start(QStringLiteral("%1 -s %2 -o - -c -z %3 -L /tmp/partclone.log").arg(executer).arg(part.filePath()).arg(Global::bufferSize), QIODevice::ReadOnly);
+            process->start(QStringLiteral("%1 -s %2 -o - -c -z %3 -L /var/log/partclone.log").arg(executer).arg(part.filePath()).arg(Global::bufferSize), QIODevice::ReadOnly);
         } else {
-            process->start(QStringLiteral("partclone.restore -s - -o %2 -z %3 -L /tmp/partclone.log").arg(part.filePath()).arg(Global::bufferSize));
+            process->start(QStringLiteral("partclone.restore -s - -o %2 -z %3 -L /var/log/partclone.log").arg(part.filePath()).arg(Global::bufferSize));
         }
 
         break;
     }
     case DDiskInfo::JsonInfo: {
diff --git a/app/src/corelib/helper.cpp b/app/src/corelib/helper.cpp
index 73e0e20..ec9f072 100644
--- a/app/src/corelib/helper.cpp
+++ b/app/src/corelib/helper.cpp
@@ -98,10 +98,12 @@ int Helper::processExec(QProcess *process, const QString &command, int timeout,
     }
 
     if (process->state() != QProcess::NotRunning) {
         dCDebug("The \"%s\" timeout, timeout: %d", qPrintable(command), timeout);
 
+        // QT Bug，某种情况下(未知) QProcess::state 返回的状态有误，导致进程已退出却未能正确获取到其当前状态
+        // 因此,额外通过系统文件判断进程是否还存在
         if (QFile::exists(QString("/proc/%1").arg(process->pid()))) {
             process->terminate();
             process->waitForFinished();
         } else {
             dCDebug("The \"%s\" is quit, but the QProcess object state is not NotRunning");
@@ -329,11 +331,11 @@ bool Helper::getPartitionSizeInfo(const QString &partDevice, qint64 *used, qint6
         if (!ok)
             return false;
 
         return true;
     } else {
-        process.start(QString("%1 -s %2 -c -q -C -L /tmp/partclone.log").arg(getPartcloneExecuter(DDevicePartInfo(partDevice))).arg(partDevice));
+        process.start(QString("%1 -s %2 -c -q -C -L /var/log/partclone.log").arg(getPartcloneExecuter(DDevicePartInfo(partDevice))).arg(partDevice));
         process.setStandardOutputFile("/dev/null");
         process.setReadChannel(QProcess::StandardError);
         process.waitForStarted();
 
         qint64 used_block = -1;
@@ -499,13 +501,13 @@ QString Helper::temporaryMountDevice(const QString &device, const QString &name,
 
     if (!mount_point.isEmpty())
         return mount_point;
 
     mount_point = "%1/.%2/mount/%3";
-    const QStringList &tmp_paths = QStandardPaths::standardLocations(QStandardPaths::TempLocation);
+    const QStringList &tmp_paths = QStandardPaths::standardLocations(QStandardPaths::RuntimeLocation);
 
-    mount_point = mount_point.arg(tmp_paths.isEmpty() ? "/tmp" : tmp_paths.first()).arg(qApp->applicationName()).arg(name);
+    mount_point = mount_point.arg(tmp_paths.isEmpty() ? "/run/user/0" : tmp_paths.first()).arg(qApp->applicationName()).arg(name);
 
     if (!QDir::current().mkpath(mount_point)) {
         dCError("mkpath \"%s\" failed", qPrintable(mount_point));
 
         return QString();
diff --git a/app/src/corelib/helper.h b/app/src/corelib/helper.h
index 19665c0..adca5a3 100644
--- a/app/src/corelib/helper.h
+++ b/app/src/corelib/helper.h
@@ -97,10 +97,12 @@ public:
 
     static QString getDeviceForFile(const QString &file, QString *rootPath = 0);
     static QString parseSerialUrl(const QString &urlString, QString *errorString = 0);
     static QString toSerialUrl(const QString &file);
 
+    static bool clearSymlink(const QString &path);
+
 signals:
     void newWarning(const QString &message);
     void newError(const QString &message);
 
 private:
diff --git a/app/src/fixboot/bootdoctor.cpp b/app/src/fixboot/bootdoctor.cpp
index 877a979..494bb04 100644
--- a/app/src/fixboot/bootdoctor.cpp
+++ b/app/src/fixboot/bootdoctor.cpp
@@ -53,12 +53,11 @@ bool BootDoctor::fix(const QString &partDevice)
         m_lastErrorString = QObject::tr("Failed to mount partition \"%1\"").arg(partDevice);
         goto failed;
     }
 
     {
-        const QStringList &tmp_paths = QStandardPaths::standardLocations(QStandardPaths::TempLocation);
-        const QString tmp_dir = (tmp_paths.isEmpty() ? "/tmp" : tmp_paths.first()) + "/.deepin-clone";
+        const QString tmp_dir = "/var/cache/deepin-clone";
 
         if (!QDir::current().mkpath(tmp_dir)) {
             dCError("mkpath \"%s\" failed", qPrintable(tmp_dir));
             goto failed;
         }
diff --git a/app/src/main.cpp b/app/src/main.cpp
index abccf6e..74be054 100644
--- a/app/src/main.cpp
+++ b/app/src/main.cpp
@@ -100,40 +100,24 @@ int main(int argc, char *argv[])
 #ifdef ENABLE_GUI
     else {
         ConsoleAppender *consoleAppender = new ConsoleAppender;
         consoleAppender->setFormat(logFormat);
 
-        RollingFileAppender *rollingFileAppender = new RollingFileAppender("/tmp/.deepin-clone.log");
+        const QString log_file("/var/log/deepin-clone.log");
+
+        RollingFileAppender *rollingFileAppender = new RollingFileAppender(log_file);
         rollingFileAppender->setFormat(logFormat);
         rollingFileAppender->setLogFilesLimit(5);
         rollingFileAppender->setDatePattern(RollingFileAppender::DailyRollover);
 
-        logger->registerAppender(consoleAppender);
         logger->registerAppender(rollingFileAppender);
+        logger->registerAppender(consoleAppender);
 
         if (qEnvironmentVariableIsSet("PKEXEC_UID")) {
             const quint32 pkexec_uid = qgetenv("PKEXEC_UID").toUInt();
-            const QDir user_home(getpwuid(pkexec_uid)->pw_dir);
-
-            QFile pam_file(user_home.absoluteFilePath(".pam_environment"));
 
-            if (pam_file.open(QIODevice::ReadOnly)) {
-                while (!pam_file.atEnd()) {
-                    const QByteArray &line = pam_file.readLine().simplified();
-
-                    if (line.startsWith("QT_SCALE_FACTOR")) {
-                        const QByteArrayList &list = line.split('=');
-
-                        if (list.count() == 2) {
-                            qputenv("QT_SCALE_FACTOR", list.last());
-                            break;
-                        }
-                    }
-                }
-
-                pam_file.close();
-            }
+            DApplication::customQtThemeConfigPathByUserHome(getpwuid(pkexec_uid)->pw_dir);
         }
 
         DApplication::loadDXcbPlugin();
         DApplication *app = new DApplication(argc, argv);
 

Vulnerable code:             process->start(QStringLiteral("%1 -s %2 -o - -c -z %3 -L /tmp/partclone.log").arg(executer).arg(part.filePath()).arg(Global::bufferSize), QIODevice::ReadOnly);
            process->start(QStringLiteral("partclone.restore -s - -o %2 -z %3 -L /tmp/partclone.log").arg(part.filePath()).arg(Global::bufferSize));
        process.start(QString("%1 -s %2 -c -q -C -L /tmp/partclone.log").arg(getPartcloneExecuter(DDevicePartInfo(partDevice))).arg(partDevice));
    const QStringList &tmp_paths = QStandardPaths::standardLocations(QStandardPaths::TempLocation);
    mount_point = mount_point.arg(tmp_paths.isEmpty() ? "/tmp" : tmp_paths.first()).arg(qApp->applicationName()).arg(name);
        const QStringList &tmp_paths = QStandardPaths::standardLocations(QStandardPaths::TempLocation);
        const QString tmp_dir = (tmp_paths.isEmpty() ? "/tmp" : tmp_paths.first()) + "/.deepin-clone";
        RollingFileAppender *rollingFileAppender = new RollingFileAppender("/tmp/.deepin-clone.log");
        logger->registerAppender(consoleAppender);
            const QDir user_home(getpwuid(pkexec_uid)->pw_dir);

            QFile pam_file(user_home.absoluteFilePath(".pam_environment"));
            if (pam_file.open(QIODevice::ReadOnly)) {
                while (!pam_file.atEnd()) {
                    const QByteArray &line = pam_file.readLine().simplified();

                    if (line.startsWith("QT_SCALE_FACTOR")) {
                        const QByteArrayList &list = line.split('=');

                        if (list.count() == 2) {
                            qputenv("QT_SCALE_FACTOR", list.last());
                            break;
                        }
                    }
                }

                pam_file.close();
            }
Fixed code:             process->start(QStringLiteral("%1 -s %2 -o - -c -z %3 -L /var/log/partclone.log").arg(executer).arg(part.filePath()).arg(Global::bufferSize), QIODevice::ReadOnly);
            process->start(QStringLiteral("partclone.restore -s - -o %2 -z %3 -L /var/log/partclone.log").arg(part.filePath()).arg(Global::bufferSize));
        // QT Bug，某种情况下(未知) QProcess::state 返回的状态有误，导致进程已退出却未能正确获取到其当前状态
        // 因此,额外通过系统文件判断进程是否还存在
        process.start(QString("%1 -s %2 -c -q -C -L /var/log/partclone.log").arg(getPartcloneExecuter(DDevicePartInfo(partDevice))).arg(partDevice));
    const QStringList &tmp_paths = QStandardPaths::standardLocations(QStandardPaths::RuntimeLocation);
    mount_point = mount_point.arg(tmp_paths.isEmpty() ? "/run/user/0" : tmp_paths.first()).arg(qApp->applicationName()).arg(name);
    static bool clearSymlink(const QString &path);

        const QString tmp_dir = "/var/cache/deepin-clone";
        const QString log_file("/var/log/deepin-clone.log");

        RollingFileAppender *rollingFileAppender = new RollingFileAppender(log_file);
        logger->registerAppender(consoleAppender);
            DApplication::customQtThemeConfigPathByUserHome(getpwuid(pkexec_uid)->pw_dir);