Commit 5d3e71b1 authored by David Faure's avatar David Faure

Security: remove support for $(...) in config keys with [$e] marker.

Summary:
It is very unclear at this point what a valid use case for this feature
would possibly be. The old documentation only mentions $(hostname) as
an example, which can be done with $HOSTNAME instead.

Note that $(...) is still supported in Exec lines of desktop files,
this does not require [$e] anyway (and actually works better without it,
otherwise the $ signs need to be doubled to obey kconfig $e escaping rules...).

Test Plan:
ctest passes; various testcases with $(...) in desktop files,
directory files, and config files, no longer execute commands.

Reviewers: mdawson, aacid, broulik, davidedmundson, kossebau, apol, sitter, security-team

Reviewed By: mdawson, davidedmundson

Subscribers: ZaWertun, rikmills, fvogt, ngraham, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D22979
parent 8e82d5ac
......@@ -38,7 +38,7 @@
#include <utime.h>
#endif
#ifndef Q_OS_WIN
#include <unistd.h> // gethostname
#include <unistd.h> // getuid
#endif
KCONFIGGROUP_DECLARE_ENUM_QOBJECT(KConfigTest, Testing)
......@@ -546,14 +546,8 @@ void KConfigTest::testPath()
QCOMPARE(group.readPathEntry("withBraces", QString()), QString("file://" + HOMEPATH));
QVERIFY(group.hasKey("URL"));
QCOMPARE(group.readEntry("URL", QString()), QString("file://" + HOMEPATH));
#if !defined(Q_OS_WIN32) && !defined(Q_OS_MAC)
// I don't know if this will work on windows
// This test hangs on OS X
QVERIFY(group.hasKey("hostname"));
char hostname[256];
QVERIFY(::gethostname(hostname, sizeof(hostname)) == 0);
QCOMPARE(group.readEntry("hostname", QString()), QString::fromLatin1(hostname));
#endif
QCOMPARE(group.readEntry("hostname", QString()), QStringLiteral("(hostname)")); // the $ got removed because empty var name
QVERIFY(group.hasKey("noeol"));
QCOMPARE(group.readEntry("noeol", QString()), QString("foo"));
......
......@@ -67,18 +67,15 @@ environment variables (and `XDG_CONFIG_HOME` in particular).
Shell Expansion
---------------
If an entry is marked with `$e`, environment variables and shell commands will
be expanded.
If an entry is marked with `$e`, environment variables will be expanded.
Name[$e]=$USER
Host[$e]=$(hostname)
When the "Name" entry is read `$USER` will be replaced with the value of the
`$USER` environment variable, and `$(hostname)` will be replaced with the output
of the `hostname` command.
`$USER` environment variable.
Note that the application will replace `$USER` and `$(hostname)` with their
respective expanded values after saving. To prevent this combine the `$e` option
Note that the application will replace `$USER` with its
expanded value after saving. To prevent this combine the `$e` option
with `$i` (immmutable) option. For example:
Name[$ei]=$USER
......
......@@ -28,19 +28,6 @@
#include <cstdlib>
#include <fcntl.h>
#ifdef _MSC_VER
static inline FILE *popen(const char *cmd, const char *mode)
{
return _popen(cmd, mode);
}
static inline int pclose(FILE *stream)
{
return _pclose(stream);
}
#else
#include <unistd.h>
#endif
#include "kconfigbackend_p.h"
#include "kconfiggroup.h"
......@@ -183,29 +170,7 @@ QString KConfigPrivate::expandString(const QString &value)
int nDollarPos = aValue.indexOf(QLatin1Char('$'));
while (nDollarPos != -1 && nDollarPos + 1 < aValue.length()) {
// there is at least one $
if (aValue[nDollarPos + 1] == QLatin1Char('(')) {
int nEndPos = nDollarPos + 1;
// the next character is not $
while ((nEndPos <= aValue.length()) && (aValue[nEndPos] != QLatin1Char(')'))) {
nEndPos++;
}
nEndPos++;
QString cmd = aValue.mid(nDollarPos + 2, nEndPos - nDollarPos - 3);
QString result;
// FIXME: wince does not have pipes
#ifndef _WIN32_WCE
FILE *fs = popen(QFile::encodeName(cmd).data(), "r");
if (fs) {
QTextStream ts(fs, QIODevice::ReadOnly);
result = ts.readAll().trimmed();
pclose(fs);
}
#endif
aValue.replace(nDollarPos, nEndPos - nDollarPos, result);
nDollarPos += result.length();
} else if (aValue[nDollarPos + 1] != QLatin1Char('$')) {
if (aValue[nDollarPos + 1] != QLatin1Char('$')) {
int nEndPos = nDollarPos + 1;
// the next character is not $
QStringRef aVarName;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment