From 71554140bdaede27b95dbe4c9b5a028a83c83cce Mon Sep 17 00:00:00 2001 From: Alexander Reinholdt 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& 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& map) // Do nothing } + map.insert("mh_command", mount); + // Mount arguments. QMap global_options = globalSambaOptions(); Smb4KCustomOptions *options = Smb4KCustomOptionsManager::self()->findOptions(share); @@ -729,30 +708,7 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap& map) // bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap& 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& map) // Do nothing } + map.insert("mh_command", mount); + // Mount arguments. QMap 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 @@ -43,12 +44,35 @@ #include #include +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(); -- cgit v0.11.2