1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147
|
From 5d3e71b1d2ecd2cb2f910036e614ffdfc895aa22 Mon Sep 17 00:00:00 2001
From: David Faure <faure@kde.org>
Date: Wed, 7 Aug 2019 09:35:36 +0200
Subject: 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
---
autotests/kconfigtest.cpp | 10 ++--------
docs/options.md | 11 ++++-------
src/core/kconfig.cpp | 37 +------------------------------------
3 files changed, 7 insertions(+), 51 deletions(-)
diff --git a/autotests/kconfigtest.cpp b/autotests/kconfigtest.cpp
index 410b5b8..9af3b46 100644
--- a/autotests/kconfigtest.cpp
+++ b/autotests/kconfigtest.cpp
@@ -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"));
diff --git a/docs/options.md b/docs/options.md
index c634c00..4a6e9bc 100644
--- a/docs/options.md
+++ b/docs/options.md
@@ -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
diff --git a/src/core/kconfig.cpp b/src/core/kconfig.cpp
index e1b11ed..f6824ce 100644
--- a/src/core/kconfig.cpp
+++ b/src/core/kconfig.cpp
@@ -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;
--
cgit v1.1
|