commit: 5040d7013b30a0a7916695c5101401607dbbe71b Author: Andreas Sturmlechner <asturm <AT> gentoo <DOT> org> AuthorDate: Sat Jun 9 07:18:17 2018 +0000 Commit: Andreas Sturmlechner <asturm <AT> gentoo <DOT> org> CommitDate: Sat Jun 9 14:18:21 2018 +0000 URL: https://gitweb.gentoo.org/proj/kde.git/commit/?id=5040d701
kde-frameworks/ktexteditor: Fix CVE-2018-10361 Package-Manager: Portage-2.3.40, Repoman-2.3.9 .../files/ktexteditor-5.46.0-CVE-2018-10361.patch | 187 +++++++++++++++++++++ .../ktexteditor/ktexteditor-5.47.0.ebuild | 2 + 2 files changed, 189 insertions(+) diff --git a/kde-frameworks/ktexteditor/files/ktexteditor-5.46.0-CVE-2018-10361.patch b/kde-frameworks/ktexteditor/files/ktexteditor-5.46.0-CVE-2018-10361.patch new file mode 100644 index 0000000000..d3b9b5d480 --- /dev/null +++ b/kde-frameworks/ktexteditor/files/ktexteditor-5.46.0-CVE-2018-10361.patch @@ -0,0 +1,187 @@ +From c81af5aa1d4f6e0f8c44b2e85ca007ba2a1e4590 Mon Sep 17 00:00:00 2001 +From: Christoph Cullmann <cullm...@kde.org> +Date: Thu, 7 Jun 2018 16:12:25 +0200 +Subject: CVE-2018-10361: privilege escalation + +improve handling of temporary file to avoid possible race-condition + +Differential Revision: https://phabricator.kde.org/D12513 +--- + src/buffer/katesecuretextbuffer.cpp | 99 +++++++++++++++++-------------------- + src/buffer/katesecuretextbuffer_p.h | 4 -- + 2 files changed, 46 insertions(+), 57 deletions(-) + +diff --git a/src/buffer/katesecuretextbuffer.cpp b/src/buffer/katesecuretextbuffer.cpp +index 0647bee..c014608 100644 +--- a/src/buffer/katesecuretextbuffer.cpp ++++ b/src/buffer/katesecuretextbuffer.cpp +@@ -53,39 +53,37 @@ ActionReply SecureTextBuffer::savefile(const QVariantMap &args) + bool SecureTextBuffer::saveFileInternal(const QString &sourceFile, const QString &targetFile, + const QByteArray &checksum, const uint ownerId, const uint groupId) + { +- QFileInfo targetFileInfo(targetFile); +- if (!QDir::setCurrent(targetFileInfo.dir().path())) { ++ /** ++ * open source file for reading ++ * if not possible, signal error ++ */ ++ QFile readFile(sourceFile); ++ if (!readFile.open(QIODevice::ReadOnly)) { + return false; + } + +- // get information about target file +- const QString targetFileName = targetFileInfo.fileName(); +- targetFileInfo.setFile(targetFileName); +- const bool newFile = !targetFileInfo.exists(); +- +- // open source and target file +- QFile readFile(sourceFile); +- //TODO use QSaveFile for saving contents and automatic atomic move on commit() when QSaveFile's security problem +- // (default temporary file permissions) is fixed +- // +- // We will first generate temporary filename and then use it relatively to prevent an attacker +- // to trick us to write contents to a different file by changing underlying directory. +- QTemporaryFile tempFile(targetFileName); ++ /** ++ * construct file info for target file ++ * we need to know things like path/exists/permissions ++ */ ++ const QFileInfo targetFileInfo(targetFile); ++ ++ /** ++ * create temporary file in current directory to be able to later do an atomic rename ++ * we need to pass full path, else QTemporaryFile uses the temporary directory ++ * if not possible, signal error, this catches e.g. a non-existing target directory, too ++ */ ++ QTemporaryFile tempFile(targetFileInfo.absolutePath() + QStringLiteral("/secureXXXXXX")); + if (!tempFile.open()) { + return false; + } +- tempFile.close(); +- QString tempFileName = QFileInfo(tempFile).fileName(); +- tempFile.setFileName(tempFileName); +- if (!readFile.open(QIODevice::ReadOnly) || !tempFile.open()) { +- return false; +- } +- const int tempFileDescriptor = tempFile.handle(); + +- // prepare checksum maker ++ /** ++ * copy contents + do checksumming ++ * if not possible, signal error ++ */ + QCryptographicHash cryptographicHash(checksumAlgorithm); +- +- // copy contents ++ const qint64 bufferLength = 4096; + char buffer[bufferLength]; + qint64 read = -1; + while ((read = readFile.read(buffer, bufferLength)) > 0) { +@@ -95,30 +93,43 @@ bool SecureTextBuffer::saveFileInternal(const QString &sourceFile, const QString + } + } + +- // check that copying was successful and checksum matched +- QByteArray localChecksum = cryptographicHash.result(); +- if (read == -1 || localChecksum != checksum || !tempFile.flush()) { ++ /** ++ * check that copying was successful and checksum matched ++ * we need to flush the file, as QTemporaryFile keeps the handle open ++ * and we later do things like renaming of the file! ++ * if not possible, signal error ++ */ ++ if ((read == -1) || (cryptographicHash.result() != checksum) || !tempFile.flush()) { + return false; + } + +- tempFile.close(); +- +- if (newFile) { ++ /** ++ * try to preserve the permissions ++ */ ++ if (!targetFileInfo.exists()) { + // ensure new file is readable by anyone + tempFile.setPermissions(tempFile.permissions() | QFile::Permission::ReadGroup | QFile::Permission::ReadOther); + } else { + // ensure the same file permissions + tempFile.setPermissions(targetFileInfo.permissions()); ++ + // ensure file has the same owner and group as before +- setOwner(tempFileDescriptor, ownerId, groupId); ++ setOwner(tempFile.handle(), ownerId, groupId); + } + +- // rename temporary file to the target file +- if (moveFile(tempFileName, targetFileName)) { ++ /** ++ * try to (atomic) rename temporary file to the target file ++ */ ++ if (moveFile(tempFile.fileName(), targetFileInfo.filePath())) { + // temporary file was renamed, there is nothing to remove anymore + tempFile.setAutoRemove(false); + return true; + } ++ ++ /** ++ * we failed ++ * QTemporaryFile will handle cleanup ++ */ + return false; + } + +@@ -141,28 +152,10 @@ bool SecureTextBuffer::moveFile(const QString &sourceFile, const QString &target + { + #if !defined(Q_OS_WIN) && !defined(Q_OS_ANDROID) + const int result = std::rename(QFile::encodeName(sourceFile).constData(), QFile::encodeName(targetFile).constData()); +- if (result == 0) { +- syncToDisk(QFile(targetFile).handle()); +- return true; +- } +- return false; ++ return (result == 0); + #else + // use racy fallback for windows + QFile::remove(targetFile); + return QFile::rename(sourceFile, targetFile); + #endif + } +- +-void SecureTextBuffer::syncToDisk(const int fd) +-{ +-#ifndef Q_OS_WIN +-#if HAVE_FDATASYNC +- fdatasync(fd); +-#else +- fsync(fd); +-#endif +-#else +- // no-op for windows +-#endif +-} +- +diff --git a/src/buffer/katesecuretextbuffer_p.h b/src/buffer/katesecuretextbuffer_p.h +index a38285b..e00721c 100644 +--- a/src/buffer/katesecuretextbuffer_p.h ++++ b/src/buffer/katesecuretextbuffer_p.h +@@ -56,8 +56,6 @@ public: + static const QCryptographicHash::Algorithm checksumAlgorithm = QCryptographicHash::Algorithm::Sha512; + + private: +- static const qint64 bufferLength = 4096; +- + /** + * Saves file contents using sets permissions. + */ +@@ -66,8 +64,6 @@ private: + + static bool moveFile(const QString &sourceFile, const QString &targetFile); + +- static void syncToDisk(const int fd); +- + public Q_SLOTS: + /** + * KAuth action to perform both prepare or move work based on given parameters. +-- +cgit v0.11.2 diff --git a/kde-frameworks/ktexteditor/ktexteditor-5.47.0.ebuild b/kde-frameworks/ktexteditor/ktexteditor-5.47.0.ebuild index 7683d63307..6920f6f7c3 100644 --- a/kde-frameworks/ktexteditor/ktexteditor-5.47.0.ebuild +++ b/kde-frameworks/ktexteditor/ktexteditor-5.47.0.ebuild @@ -46,6 +46,8 @@ DEPEND="${RDEPEND} RESTRICT+=" test" +PATCHES=( "${FILESDIR}/${P}-CVE-2018-10361.patch" ) + src_configure() { local mycmakeargs=( $(cmake-utils_use_find_package editorconfig EditorConfig)
