[Kde-accessibility] Security patch for command plugin

Olaf Jan Schmidt olaf@amen-online.de
Sat, 7 Dec 2002 00:56:45 +0100


--------------Boundary-00=_LI2Q24P4LNR13F656BNT
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi!

After hearing the reason why KDE 3.1 is delayed, I realized that we might=
=20
have a similar problem in the command plug-in.

I had a phone conversation with Gunnar about that today, and Gunnar=20
confirmed that there is a bug in the command line evaluation. Variables=20
are always replaced with quoted values, even if they are already in=20
quotes. The result is that the variables will be unquoted, leading to the=
=20
security hole that someone who can get the user to send a specially=20
prepared text like "; rm -r *; echo" to proklam can cause a lot of=20
damage.

I also simplyfied the variable substitution.

If there are no problems with the patch, I will commit on Sunday.

Olaf.

- --=20
Olaf Jan Schmidt, KDE Accessibility Project

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iEYEARECAAYFAj3xOUYACgkQoLYC8AehV8dLNgCgh2WQBmUJo+ZeW4MA+QGI+Nx4
08IAn2ljg1diCmS18wXrtDW1zCEVVQs8
=3D0SCe
-----END PGP SIGNATURE-----

--------------Boundary-00=_LI2Q24P4LNR13F656BNT
Content-Type: text/x-diff;
  charset="us-ascii";
  name="command-speech.cpp.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="command-speech.cpp.diff"

--- kdenonbeta/proklam/plugins/command/speech.cpp.old	Fri Nov 29 09:19:04 2002
+++ kdenonbeta/proklam/plugins/command/speech.cpp	Fri Dec  6 23:54:41 2002
@@ -52,36 +52,38 @@
       QString escText = KShellProcess::quote(encText);
 
       // 2. prepare the command:
-      // 2.a) split the command into pieces
-      QStringList commandList = QStringList::split ("%", command, true);
-      
-      // 2.b) analyze the piece boundaries
-      bool escaped = !command.startsWith ("%");
-      command = "";
-      QStringList::iterator it;
-      for (it = commandList.begin(); it != commandList.end(); ++it) {
-         if (escaped) {
-            // "%%"
-            command += *it;
-            escaped = false;
-         }
-         else {
-            if ((*it).isNull() || (*it).isEmpty())
-               // "%" followed end of string or another "%"
-               escaped = true;
-            else if ((*it).startsWith("t")) {
-               // "%t"
-               command += escText;
-               command += (*it).right ((*it).length()-1);
-            }
-            else
-               // "%" plus any other character
-               command += *it;
-         }
+      // 2.a) unquote variables (they will be replaced with quoted values later)
+      bool issinglequote=false;
+      bool isdoublequote=false;
+      QRegExp	re_noquote("(\"|\')");
+      QRegExp	re_singlequote("(\'|%t)");
+      QRegExp	re_doublequote("(\"|%t)");
+      for ( int i = re_noquote.search(command);
+            i != -1;
+            i = (issinglequote?re_singlequote.search(command,i+1)
+                :isdoublequote?re_doublequote.search(command,i+1)
+                :re_noquote.search(command,i+1))
+      )
+      {   if (command[i]=='\'')
+             issinglequote=!issinglequote;
+          else if (command[i]=='"')
+             isdoublequote=!isdoublequote;
+          else if (issinglequote)
+          {  command.insert (i+re_singlequote.matchedLength(), "'");
+             command.insert (i, "'");
+             i+=re_singlequote.matchedLength()+1;
+          }
+          else if (isdoublequote)
+          {  command.insert (i+re_doublequote.matchedLength(), '"');
+             command.insert (i, '"');
+             i+=re_doublequote.matchedLength()+1;
+          }
       }
-      if (escaped)
-         command = command + "%";
-      
+
+      // 2.b) replace variables with quoted values
+      while (command.find("%t") != -1)
+            command.replace(QRegExp("%t"), escText);
+
       // 3. create a new process
       process << command;
       connect(&process, SIGNAL(processExited(KProcess *)), this, SLOT(processExited(KProcess *)));

--------------Boundary-00=_LI2Q24P4LNR13F656BNT--