From: Alexander Reinholdt <alexander.reinholdt@kdemail.net>
Date: Wed, 10 May 2017 10:23:34 +0200
Subject: Find the mount/umount commands in the helper

Instead of trusting what we get passed in
CVE-2017-8849
---
 core/smb4kglobal.cpp         | 65 +++++++++++++++++++++++++++++++++++-
 core/smb4kglobal.h           | 16 ++++++++-
 core/smb4kmounter_p.cpp      | 78 ++++----------------------------------------
 helpers/CMakeLists.txt       |  6 +++-
 helpers/smb4kmounthelper.cpp | 51 +++++++++++++++++++++++++++--
 5 files changed, 139 insertions(+), 77 deletions(-)

diff --git a/core/smb4kglobal.cpp b/core/smb4kglobal.cpp
index 172016f..818a78a 100644
--- a/core/smb4kglobal.cpp
+++ b/core/smb4kglobal.cpp
@@ -2,7 +2,7 @@
     smb4kglobal  -  This is the global namespace for Smb4K.
                              -------------------
     begin                : Sa Apr 2 2005
-    copyright            : (C) 2005-2014 by Alexander Reinholdt
+    copyright            : (C) 2005-2017 by Alexander Reinholdt
     email                : alexander.reinholdt@kdemail.net
  ***************************************************************************/
 
@@ -851,3 +851,66 @@ QStringList Smb4KGlobal::whitelistedMountArguments()
 #endif
 
 
+const QString Smb4KGlobal::findMountExecutable()
+{
+  QString mount;
+  QStringList paths;
+  paths << "/bin";
+  paths << "/sbin";
+  paths << "/usr/bin";
+  paths << "/usr/sbin";
+  paths << "/usr/local/bin";
+  paths << "/usr/local/sbin";
+
+  for (int i = 0; i < paths.size(); ++i)
+  {
+#if defined(Q_OS_LINUX)
+    mount = KGlobal::dirs()->findExe("mount.cifs", paths.at(i));
+#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
+    mount = KGlobal::dirs()->findExe("mount_smbfs", paths.at(i));
+#endif
+
+    if (!mount.isEmpty())
+    {
+      break;
+    }
+    else
+    {
+      continue;
+    }
+  }
+  
+  return mount;
+}
+
+
+const QString Smb4KGlobal::findUmountExecutable()
+{
+  // Find the umount program.
+  QString umount;
+  QStringList paths;
+  paths << "/bin";
+  paths << "/sbin";
+  paths << "/usr/bin";
+  paths << "/usr/sbin";
+  paths << "/usr/local/bin";
+  paths << "/usr/local/sbin";
+
+  for ( int i = 0; i < paths.size(); ++i )
+  {
+    umount = KGlobal::dirs()->findExe("umount", paths.at(i));
+
+    if (!umount.isEmpty())
+    {
+      break;
+    }
+    else
+    {
+      continue;
+    }
+  }
+  
+  return umount;
+}
+
+
diff --git a/core/smb4kglobal.h b/core/smb4kglobal.h
index db1805b..0ef377d 100644
--- a/core/smb4kglobal.h
+++ b/core/smb4kglobal.h
@@ -2,7 +2,7 @@
     smb4kglobal  -  This is the global namespace for Smb4K.
                              -------------------
     begin                : Sa Apr 2 2005
-    copyright            : (C) 2005-2014 by Alexander Reinholdt
+    copyright            : (C) 2005-2017 by Alexander Reinholdt
     email                : alexander.reinholdt@kdemail.net
  ***************************************************************************/
 
@@ -455,6 +455,20 @@ namespace Smb4KGlobal
    */
   KDE_EXPORT QStringList whitelistedMountArguments();
 #endif
+  
+  /**
+   * Find the mount executable on the system.
+   * 
+   * @returns the path of the mount executable.
+   */
+  KDE_EXPORT const QString findMountExecutable();
+  
+  /**
+   * Find the umount executable on the system.
+   * 
+   * @returns the path of the umount executable.
+   */
+  KDE_EXPORT const QString findUmountExecutable();
 };
 
 #endif
diff --git a/core/smb4kmounter_p.cpp b/core/smb4kmounter_p.cpp
index 63a87ed..342052a 100644
--- a/core/smb4kmounter_p.cpp
+++ b/core/smb4kmounter_p.cpp
@@ -207,30 +207,7 @@ bool Smb4KMountJob::createMountAction(Smb4KShare *share, Action *action)
 //
 bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
 {
-  // Find the mount program.
-  QString mount;
-  QStringList paths;
-  paths << "/bin";
-  paths << "/sbin";
-  paths << "/usr/bin";
-  paths << "/usr/sbin";
-  paths << "/usr/local/bin";
-  paths << "/usr/local/sbin";
-
-  for (int i = 0; i < paths.size(); ++i)
-  {
-    mount = KGlobal::dirs()->findExe("mount.cifs", paths.at(i));
-
-    if (!mount.isEmpty())
-    {
-      map.insert("mh_command", mount);
-      break;
-    }
-    else
-    {
-      continue;
-    }
-  }
+  const QString mount = findMountExecutable();
 
   if (mount.isEmpty())
   {
@@ -242,6 +219,8 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
     // Do nothing
   }
   
+  map.insert("mh_command", mount);
+  
   // Mount arguments.
   QMap<QString, QString> global_options = globalSambaOptions();
   Smb4KCustomOptions *options  = Smb4KCustomOptionsManager::self()->findOptions(share);
@@ -729,30 +708,7 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
 //
 bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
 {
-  // Find the mount program.
-  QString mount;
-  QStringList paths;
-  paths << "/bin";
-  paths << "/sbin";
-  paths << "/usr/bin";
-  paths << "/usr/sbin";
-  paths << "/usr/local/bin";
-  paths << "/usr/local/sbin";
-
-  for (int i = 0; i < paths.size(); ++i)
-  {
-    mount = KGlobal::dirs()->findExe("mount_smbfs", paths.at(i));
-
-    if (!mount.isEmpty())
-    {
-      map.insert("mh_command", mount);
-      break;
-    }
-    else
-    {
-      continue;
-    }
-  }
+  const QString mount = findMountExecutable();
 
   if (mount.isEmpty())
   {
@@ -764,6 +720,8 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
     // Do nothing
   }
   
+  map.insert("mh_command", mount);
+  
   // Mount arguments.
   QMap<QString, QString> global_options = globalSambaOptions();
   Smb4KCustomOptions *options  = Smb4KCustomOptionsManager::self()->findOptions(share);
@@ -1253,29 +1211,7 @@ bool Smb4KUnmountJob::createUnmountAction(Smb4KShare *share, Action *action)
     // Do nothing
   }
   
-  // Find the umount program.
-  QString umount;
-  QStringList paths;
-  paths << "/bin";
-  paths << "/sbin";
-  paths << "/usr/bin";
-  paths << "/usr/sbin";
-  paths << "/usr/local/bin";
-  paths << "/usr/local/sbin";
-
-  for ( int i = 0; i < paths.size(); ++i )
-  {
-    umount = KGlobal::dirs()->findExe("umount", paths.at(i));
-
-    if (!umount.isEmpty())
-    {
-      break;
-    }
-    else
-    {
-      continue;
-    }
-  }
+  const QString umount = findUmountExecutable();
 
   if (umount.isEmpty() && !m_silent)
   {
diff --git a/helpers/CMakeLists.txt b/helpers/CMakeLists.txt
index e9e670b..cd4228d 100644
--- a/helpers/CMakeLists.txt
+++ b/helpers/CMakeLists.txt
@@ -1,7 +1,11 @@
+include_directories(
+  ${CMAKE_SOURCE_DIR}/core
+  ${CMAKE_BINARY_DIR}/core )
+
 set( smb4kmounthelper_SRCS smb4kmounthelper.cpp )
 
 kde4_add_executable( mounthelper ${smb4kmounthelper_SRCS} )
-target_link_libraries( mounthelper ${KDE4_KDECORE_LIBS} ${KDE4_KIO_LIBS} )
+target_link_libraries( mounthelper smb4kcore ${KDE4_KDECORE_LIBS} ${KDE4_KIO_LIBS} )
 install( TARGETS mounthelper DESTINATION ${LIBEXEC_INSTALL_DIR} )
 
 kde4_install_auth_helper_files( mounthelper net.sourceforge.smb4k.mounthelper root )
diff --git a/helpers/smb4kmounthelper.cpp b/helpers/smb4kmounthelper.cpp
index a2f2fed..7959020 100644
--- a/helpers/smb4kmounthelper.cpp
+++ b/helpers/smb4kmounthelper.cpp
@@ -29,6 +29,7 @@
 
 // application specific includes
 #include "smb4kmounthelper.h"
+#include "core/smb4kglobal.h"
 
 // Qt includes
 #include <QProcessEnvironment>
@@ -43,12 +44,35 @@
 #include <kmountpoint.h>
 #include <kurl.h>
 
+using namespace Smb4KGlobal;
+
 KDE4_AUTH_HELPER_MAIN( "net.sourceforge.smb4k.mounthelper", Smb4KMountHelper )
 
 
 ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
 {
   ActionReply reply;
+  
+  //
+  // Get the mount executable
+  //
+  const QString mount = findMountExecutable();
+  
+  //
+  // Check the executable
+  //
+  if (mount != args["mh_command"].toString())
+  {
+    // Something weird is going on, bail out.
+    reply.setErrorCode(ActionReply::HelperError);
+    reply.setErrorDescription(i18n("Wrong executable passed. Bailing out."));
+    return reply;
+  }
+  else
+  {
+    // Do nothing
+  }
+  
   // The mountpoint is a unique and can be used to
   // find the share.
   reply.addData("mh_mountpoint", args["mh_mountpoint"]);
@@ -75,12 +99,12 @@ ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
   // Set the mount command here.
   QStringList command;
 #if defined(Q_OS_LINUX)
-  command << args["mh_command"].toString();
+  command << mount;
   command << args["mh_unc"].toString();
   command << args["mh_mountpoint"].toString();
   command << args["mh_options"].toStringList();
 #elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
-  command << args["mh_command"].toString();
+  command << mount;
   command << args["mh_options"].toStringList();
   command << args["mh_unc"].toString();
   command << args["mh_mountpoint"].toString();
@@ -161,6 +185,27 @@ ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
 ActionReply Smb4KMountHelper::unmount(const QVariantMap &args)
 {
   ActionReply reply;
+  
+  //
+  // Get the umount executable
+  //
+  const QString umount = findUmountExecutable();
+  
+  //
+  // Check the executable
+  //
+  if (umount != args["mh_command"].toString())
+  {
+    // Something weird is going on, bail out.
+    reply.setErrorCode(ActionReply::HelperError);
+    reply.setErrorDescription(i18n("Wrong executable passed. Bailing out."));
+    return reply;
+  }
+  else
+  {
+    // Do nothing
+  }
+  
   // The mountpoint is a unique and can be used to
   // find the share.
   reply.addData("mh_mountpoint", args["mh_mountpoint"]);
@@ -208,7 +253,7 @@ ActionReply Smb4KMountHelper::unmount(const QVariantMap &args)
   
   // Set the umount command here.
   QStringList command;
-  command << args["mh_command"].toString();
+  command << umount;
   command << args["mh_options"].toStringList();
   command << args["mh_mountpoint"].toString();