Fixes for "vulnerable command line parsing"

Signed-off-by: Sergey G. Brester <info@sebres.de>
Reviewed-by: Tino Reichardt <milky-7zip@mcmilk.de>

CPP/Common/CommandLineParser.cpp

@@ -6,41 +6,108 @@
 
 namespace NCommandLineParser {
 
-bool SplitCommandLine(const UString &src, UString &dest1, UString &dest2)
+static const wchar_t * _SplitCommandLine(const wchar_t* s, UString &dest)
 {
-  dest1.Empty();
+  unsigned qcount = 0, bcount = 0;
-  dest2.Empty();
+  wchar_t c; const wchar_t *f, *b;
-  bool quoteMode = false;
+
-  unsigned i;
+  dest.Empty();
-  for (i = 0; i < src.Len(); i++)
+
+  // skip spaces:
+  while (isblank(*s)) { s++; };
+  b = f = s;
+
+  while ((c = *s++) != 0)
   {
-    wchar_t c = src[i];
+    switch (c)
-    if ((c == L' ' || c == L'\t') && !quoteMode)
     {
-      dest2 = src.Ptr(i + 1);
+      case L'\\':
-      return i != 0;
+        // a backslash - count them up to quote-char or regular char 
+        bcount++;
+      break;
+      case L'"':
+        // check quote char is escaped:
+        if (!(bcount & 1))
+        {
+          // preceded by an even number of '\', this is half that
+          // number of '\':
+          dest.AddFrom(f, (unsigned)(s - f - bcount/2 - 1)); f = s;
+          // count quote chars:
+          qcount++;
         }
-    if (c == L'\"')
-      quoteMode = !quoteMode;
         else
-      dest1 += c;
+        {
+          // preceded by an odd number of '\', this is half that
+          // number of '\' followed by an escaped '"':
+          dest.AddFrom(f, (unsigned)(s - f - bcount/2 - 2)); f = s;
+          dest += L'"';
         }
-  return i != 0;
+        bcount = 0;
+        // now count the number of consecutive quotes (inclusive
+        // the quote that lead us here):
+        while (*s == L'"')
+        {
+          s++;
+          if (++qcount == 3)
+          {
+            dest += L'"';
+            qcount = 0;
+          }
+        }
+        f = s;
+        if (qcount == 2)
+          qcount = 0;
+      break;
+      case L' ':
+      case L'\t':
+        // a space (end of arg or regular char):
+        if (!qcount)
+        {
+          // end of argument:
+          dest.AddFrom(f, (unsigned)(s - f - 1)); f = s;
+          // skip to the next one:
+          while (isblank(*s)) { s++; };
+          bcount = 0;
+          goto done;
+        }
+      // no break - a space as regular char:
+      default:
+        // a regular character, reset backslash counter
+        bcount = 0;
+    }
+  }
+  s--; // back to NTS-zero char
+  dest.AddFrom(f, (unsigned)(s - f));
+done:
+  // remaining part if argument was found, otherwise NULL:
+  return (dest.Len() || *b) ? s : NULL;
 }
 
-void SplitCommandLine(const UString &s, UStringVector &parts)
+bool SplitCommandLine(const UString& src, UString& dest1, UString& dest2)
 {
-  UString sTemp (s);
+  const wchar_t *s = src.Ptr();
-  sTemp.Trim();
+  s = _SplitCommandLine(s, dest1);
+  if (s) {
+    dest2 = s;
+    return true;
+  } else {
+    dest2.Empty();
+    return false;
+  }
+}
+
+void SplitCommandLine(const UString &src, UStringVector &parts)
+{
+  const wchar_t *s = src.Ptr();
   parts.Clear();
   for (;;)
   {
-    UString s1, s2;
+    UString s1;
-    if (SplitCommandLine(sTemp, s1, s2))
+    s = _SplitCommandLine(s, s1);
+    if (s)
       parts.Add(s1);
-    if (s2.IsEmpty())
+    if (!s || !*s)
       break;
-    sTemp = s2;
   }
 }
 

CPP/Common/MyString.cpp

@@ -1206,6 +1206,16 @@ UString &UString::operator=(const UString &s)
   return *this;
 }
 
+void UString::AddFrom(const wchar_t *s, unsigned len) // no check
+{
+  if (len) {
+    Grow(len);
+    wmemcpy(_chars + _len, s, len);
+    _len += len;
+    _chars[_len] = 0;
+  }
+}
+
 void UString::SetFrom(const wchar_t *s, unsigned len) // no check
 {
   if (len > _limit)

CPP/Common/MyString.h

@@ -628,6 +628,7 @@ public:
   UString &operator=(char c) { return (*this)=((wchar_t)(unsigned char)c); }
   UString &operator=(const wchar_t *s);
   UString &operator=(const UString &s);
+  void AddFrom(const wchar_t *s, unsigned len); // no check
   void SetFrom(const wchar_t *s, unsigned len); // no check
   void SetFromBstr(LPCOLESTR s);
   UString &operator=(const char *s);