CVE-2017-1000203: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: ROOT version 6.9.03 and below is vulnerable to an authenticated shell metacharacter injection in the rootd daemon resulting in remote code execution
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2017-11-17T15:29Z
Repository: https://github.com/root-project/root
Language: Other
Files changed: net/rootd/src/rootd.cxx
Commit date: 2017-04-19 17:31:15 +0200
Commit message: rootd: do not use popen() to expand path names (#513)

Addressed security threat reported by S. Luders.
Using the same technology used in TSystem::ExpandFileName (with clang-format processing).
Diff: diff --git a/net/rootd/src/rootd.cxx b/net/rootd/src/rootd.cxx
index 14e185cc8e..56bbeec291 100644
--- a/net/rootd/src/rootd.cxx
+++ b/net/rootd/src/rootd.cxx
@@ -380,33 +380,10 @@ void ErrSys(int level,const char *msg, int size)
 
 const char *shellMeta   = "~*[]{}?$";
 const char *shellStuff  = "(){}<>\"'";
 const char  shellEscape = '\\';
 
-////////////////////////////////////////////////////////////////////////////////
-/// Escape specchars in src with escchar and copy to dst.
-
-static int EscChar(const char *src, char *dst, int dstlen, const char *specchars, char escchar)
-{
-   const char *p;
-   char *q, *end = dst+dstlen-1;
-
-   for (p = src, q = dst; *p && q < end; ) {
-      if (strchr(specchars, *p)) {
-         *q++ = escchar;
-         if (q < end)
-            *q++ = *p++;
-      } else
-         *q++ = *p++;
-   }
-   *q = '\0';
-
-   if (*p != 0)
-      return -1;
-   return q-dst;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// After SO_KEEPALIVE times out we probably get a SIGPIPE.
 
 void SigPipe(int)
 {
@@ -445,100 +422,200 @@ static const char *HomeDirectory(const char *name)
    }
    return 0;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Expand a pathname getting rid of special shell characters like ~.$, etc.
-/// Returned string must be freed by caller.
+/// Returns the current working directory.
 
-char *RootdExpandPathName(const char *name)
+static const char *WorkingDirectory()
 {
-   const char *patbuf = name;
+   static char path[kMAXPATHLEN];
 
-   // skip leading blanks
-   while (*patbuf == ' ')
-      patbuf++;
-
-   // any shell meta characters?
-   for (const char *p = patbuf; *p; p++)
-      if (strchr(shellMeta, *p))
-         goto needshell;
+   if (getcwd(path, kMAXPATHLEN)) return path;
+   return 0;
+}
 
-   return strdup(name);
+////////////////////////////////////////////////////////////////////////////
+/// Method for pathname expansion.
+/// Returns kTRUE in case of error and kFALSE otherwise.
 
-needshell:
-   // escape shell quote characters
-   char escPatbuf[kMAXPATHLEN];
-   EscChar(patbuf, escPatbuf, sizeof(escPatbuf), shellStuff, shellEscape);
+Bool_t RootdExpandFileName(const char *fname, char *xname, const int kBufSize)
+{
+   int n, ier, iter, lx, ncopy;
+   char *inp, *out, *x, *t, buff[kBufSize * 4];
+   const char *b, *c, *e;
+   const char *p;
 
-   char cmd[kMAXPATHLEN];
-#ifdef __hpux
-   strlcpy(cmd, "/bin/echo ", sizeof(cmd));
-#else
-   strlcpy(cmd, "echo ", sizeof(cmd));
-#endif
+   iter = 0;
+   xname[0] = 0;
+   inp = buff + kBufSize;
+   out = inp + kBufSize;
+   inp[-1] = ' ';
+   inp[0] = 0;
+   out[-1] = ' ';
+   c = fname + strspn(fname, " \t\f\r");
+   // VP  if (isalnum(c[0])) { strcpy(inp, WorkingDirectory()); strcat(inp, "/"); } // add $cwd
 
-   // emulate csh -> popen executes sh
-   if (escPatbuf[0] == '~') {
-      const char *hd;
-      if (escPatbuf[1] != '\0' && escPatbuf[1] != '/') {
-         // extract user name
-         char uname[70], *p, *q;
-         for (p = &escPatbuf[1], q = uname; *p && *p !='/';)
-            *q++ = *p++;
-         *q = '\0';
-         hd = HomeDirectory(uname);
-         if (hd == 0)
-            strcat(cmd, escPatbuf);
-         else {
-            strcat(cmd, hd);
-            strcat(cmd, p);
-         }
+   strncat(inp, c, kBufSize - strlen(inp) - 1);
 
+again:
+   iter++;
+   c = inp;
+   ier = 0;
+   x = out;
+   x[0] = 0;
+
+   p = 0;
+   e = 0;
+   if (c[0] == '~' && c[1] == '/') { // ~/ case
+      std::string hd = HomeDirectory(0);
+      p = hd.c_str();
+      e = c + 1;
+      if (p) { // we have smth to copy
+         strlcpy(x, p, kBufSize);
+         x += strlen(p);
+         c = e;
       } else {
-         hd = HomeDirectory(0);
-         if (hd == 0) {
-            Error(ErrSys, kErrFatal, "RootdExpandPathName: no home directory");
-            return 0;
+         ++ier;
+         ++c;
+      }
+   } else if (c[0] == '~' && c[1] != '/') { // ~user case
+      n = strcspn(c + 1, "/ ");
+      buff[0] = 0;
+      strncat(buff, c + 1, n);
+      std::string hd = HomeDirectory(buff);
+      e = c + 1 + n;
+      if (!hd.empty()) { // we have smth to copy
+         p = hd.c_str();
+         strlcpy(x, p, kBufSize);
+         x += strlen(p);
+         c = e;
+      } else {
+         x++ [0] = c[0];
+         //++ier;
+         ++c;
+      }
+   }
+
+   for (; c[0]; c++) {
+
+      p = 0;
+      e = 0;
+
+      if (c[0] == '.' && c[1] == '/' && c[-1] == ' ') { // $cwd
+         std::string wd = WorkingDirectory();
+         strlcpy(buff, wd.c_str(), kBufSize);
+         p = buff;
+         e = c + 1;
+      }
+      if (p) { // we have smth to copy */
+         strlcpy(x, p, kBufSize);
+         x += strlen(p);
+         c = e - 1;
+         continue;
+      }
+
+      if (c[0] != '$') { // not $, simple copy
+         x++ [0] = c[0];
+      } else { // we have a $
+         b = c + 1;
+         if (c[1] == '(') b++;
+         if (c[1] == '{') b++;
+         if (b[0] == '$')
+            e = b + 1;
+         else
+            for (e = b; isalnum(e[0]) || e[0] == '_'; e++)
+               ;
+         buff[0] = 0;
+         strncat(buff, b, e - b);
+         p = getenv(buff);
+         if (!p) { // too bad, try UPPER case
+            for (t = buff; (t[0] = toupper(t[0])); t++)
+               ;
+            p = getenv(buff);
+         }
+         if (!p) { // too bad, try Lower case
+            for (t = buff; (t[0] = tolower(t[0])); t++)
+               ;
+            p = getenv(buff);
+         }
+         if (!p && !strcmp(buff, "cwd")) { // it is $cwd
+            std::string wd = WorkingDirectory();
+            strlcpy(buff, wd.c_str(), kBufSize);
+            p = buff;
+         }
+         if (!p && !strcmp(buff, "$")) { // it is $$ (replace by getpid())
+            snprintf(buff, kBufSize * 4, "%d", (int)getpid());
+            p = buff;
+         }
+         if (!p) { // too bad, nothing can help
+            ier++;
+            x++ [0] = c[0];
+         } else { // It is OK, copy result
+            int lp = strlen(p);
+            if (lp >= kBufSize) {
+               // make sure lx will be >= kBufSize (see below)
+               strlcpy(x, p, kBufSize);
+               x += kBufSize;
+               break;
+            }
+            strcpy(x, p);
+            x += lp;
+            c = (b == c + 1) ? e - 1 : e;
          }
-         strcat(cmd, hd);
-         strcat(cmd, &escPatbuf[1]);
       }
-   } else
-      strcat(cmd, escPatbuf);
+   }
 
-   FILE *pf;
-   if ((pf = ::popen(&cmd[0], "r")) == 0) {
-      Error(ErrSys, kErrFatal, "RootdExpandPathName: error in popen(%s)", cmd);
-      return 0;
+   x[0] = 0;
+   lx = x - out;
+   if (ier && iter < 3) {
+      strlcpy(inp, out, kBufSize);
+      goto again;
    }
+   ncopy = (lx >= kBufSize) ? kBufSize - 1 : lx;
+   xname[0] = 0;
+   strncat(xname, out, ncopy);
 
-   // read first argument
-   char expPatbuf[kMAXPATHLEN];
-   int  ch, i, cnt = 0;
-again:
-   for (i = 0, ch = fgetc(pf); ch != EOF && ch != ' ' && ch != '\n'; i++, ch = fgetc(pf)) {
-      expPatbuf[i] = ch;
-      cnt++;
+   if (ier || ncopy != lx) {
+      Error(ErrFatal, kErrFatal, "RootdExpandFileName: fatal error:\n\t input: %s\n\t output: %s", fname, xname);
+      return true;
    }
-   // this will be true if forked process was not yet ready to be read
-   if (cnt == 0 && ch == EOF) goto again;
-   expPatbuf[cnt] = '\0';
-
-   // skip rest of pipe
-   while (ch != EOF) {
-      ch = fgetc(pf);
-      if (ch == ' ' || ch == '\t') {
-         ::pclose(pf);
-         Error(ErrFatal, kErrFatal, "RootdExpandPathName: expression ambigous");
+
+   return false;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Expand a pathname getting rid of special shell characters like ~.$, etc.
+/// Returned string must be freed by caller.
+
+char *RootdExpandPathName(const char *name)
+{
+   const char *patbuf = name;
+
+   // skip leading blanks
+   while (*patbuf == ' ') patbuf++;
+
+   // any shell meta characters?
+   bool needesc = false;
+   for (const char *p = patbuf; *p; p++)
+      if (strchr(shellMeta, *p)) {
+         needesc = true;
+         break;
+      }
+
+   // Escape meta characters, if required
+   if (needesc) {
+      const int kBufSize = kMAXPATHLEN;
+      char xname[kBufSize];
+      if (RootdExpandFileName(name, xname, kBufSize)) {
+         Error(ErrFatal, kErrFatal, "RootdExpandPathName: problem escaping meta characters");
          return 0;
+      } else {
+         return strdup(xname);
       }
    }
-
-   ::pclose(pf);
-
-   return strdup(expPatbuf);
+   return strdup(name);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Checks gRootdTab file to see if file can be opened. If mode = 1 then
 /// check if file can safely be opened in write mode, i.e. see if file

Vulnerable code: ////////////////////////////////////////////////////////////////////////////////
/// Escape specchars in src with escchar and copy to dst.

static int EscChar(const char *src, char *dst, int dstlen, const char *specchars, char escchar)
{
   const char *p;
   char *q, *end = dst+dstlen-1;

   for (p = src, q = dst; *p && q < end; ) {
      if (strchr(specchars, *p)) {
         *q++ = escchar;
         if (q < end)
            *q++ = *p++;
      } else
         *q++ = *p++;
   }
   *q = '\0';

   if (*p != 0)
      return -1;
   return q-dst;
}

/// Expand a pathname getting rid of special shell characters like ~.$, etc.
/// Returned string must be freed by caller.
char *RootdExpandPathName(const char *name)
   const char *patbuf = name;
   // skip leading blanks
   while (*patbuf == ' ')
      patbuf++;

   // any shell meta characters?
   for (const char *p = patbuf; *p; p++)
      if (strchr(shellMeta, *p))
         goto needshell;
   return strdup(name);
needshell:
   // escape shell quote characters
   char escPatbuf[kMAXPATHLEN];
   EscChar(patbuf, escPatbuf, sizeof(escPatbuf), shellStuff, shellEscape);
   char cmd[kMAXPATHLEN];
#ifdef __hpux
   strlcpy(cmd, "/bin/echo ", sizeof(cmd));
#else
   strlcpy(cmd, "echo ", sizeof(cmd));
#endif
   // emulate csh -> popen executes sh
   if (escPatbuf[0] == '~') {
      const char *hd;
      if (escPatbuf[1] != '\0' && escPatbuf[1] != '/') {
         // extract user name
         char uname[70], *p, *q;
         for (p = &escPatbuf[1], q = uname; *p && *p !='/';)
            *q++ = *p++;
         *q = '\0';
         hd = HomeDirectory(uname);
         if (hd == 0)
            strcat(cmd, escPatbuf);
         else {
            strcat(cmd, hd);
            strcat(cmd, p);
         }
         hd = HomeDirectory(0);
         if (hd == 0) {
            Error(ErrSys, kErrFatal, "RootdExpandPathName: no home directory");
            return 0;
         strcat(cmd, hd);
         strcat(cmd, &escPatbuf[1]);
   } else
      strcat(cmd, escPatbuf);
   FILE *pf;
   if ((pf = ::popen(&cmd[0], "r")) == 0) {
      Error(ErrSys, kErrFatal, "RootdExpandPathName: error in popen(%s)", cmd);
      return 0;
   // read first argument
   char expPatbuf[kMAXPATHLEN];
   int  ch, i, cnt = 0;
again:
   for (i = 0, ch = fgetc(pf); ch != EOF && ch != ' ' && ch != '\n'; i++, ch = fgetc(pf)) {
      expPatbuf[i] = ch;
      cnt++;
   // this will be true if forked process was not yet ready to be read
   if (cnt == 0 && ch == EOF) goto again;
   expPatbuf[cnt] = '\0';

   // skip rest of pipe
   while (ch != EOF) {
      ch = fgetc(pf);
      if (ch == ' ' || ch == '\t') {
         ::pclose(pf);
         Error(ErrFatal, kErrFatal, "RootdExpandPathName: expression ambigous");

   ::pclose(pf);

   return strdup(expPatbuf);
Fixed code: /// Returns the current working directory.
static const char *WorkingDirectory()
   static char path[kMAXPATHLEN];
   if (getcwd(path, kMAXPATHLEN)) return path;
   return 0;
}
////////////////////////////////////////////////////////////////////////////
/// Method for pathname expansion.
/// Returns kTRUE in case of error and kFALSE otherwise.
Bool_t RootdExpandFileName(const char *fname, char *xname, const int kBufSize)
{
   int n, ier, iter, lx, ncopy;
   char *inp, *out, *x, *t, buff[kBufSize * 4];
   const char *b, *c, *e;
   const char *p;
   iter = 0;
   xname[0] = 0;
   inp = buff + kBufSize;
   out = inp + kBufSize;
   inp[-1] = ' ';
   inp[0] = 0;
   out[-1] = ' ';
   c = fname + strspn(fname, " \t\f\r");
   // VP  if (isalnum(c[0])) { strcpy(inp, WorkingDirectory()); strcat(inp, "/"); } // add $cwd
   strncat(inp, c, kBufSize - strlen(inp) - 1);
again:
   iter++;
   c = inp;
   ier = 0;
   x = out;
   x[0] = 0;

   p = 0;
   e = 0;
   if (c[0] == '~' && c[1] == '/') { // ~/ case
      std::string hd = HomeDirectory(0);
      p = hd.c_str();
      e = c + 1;
      if (p) { // we have smth to copy
         strlcpy(x, p, kBufSize);
         x += strlen(p);
         c = e;
         ++ier;
         ++c;
      }
   } else if (c[0] == '~' && c[1] != '/') { // ~user case
      n = strcspn(c + 1, "/ ");
      buff[0] = 0;
      strncat(buff, c + 1, n);
      std::string hd = HomeDirectory(buff);
      e = c + 1 + n;
      if (!hd.empty()) { // we have smth to copy
         p = hd.c_str();
         strlcpy(x, p, kBufSize);
         x += strlen(p);
         c = e;
      } else {
         x++ [0] = c[0];
         //++ier;
         ++c;
      }
   }

   for (; c[0]; c++) {

      p = 0;
      e = 0;

      if (c[0] == '.' && c[1] == '/' && c[-1] == ' ') { // $cwd
         std::string wd = WorkingDirectory();
         strlcpy(buff, wd.c_str(), kBufSize);
         p = buff;
         e = c + 1;
      }
      if (p) { // we have smth to copy */
         strlcpy(x, p, kBufSize);
         x += strlen(p);
         c = e - 1;
         continue;
      }

      if (c[0] != '$') { // not $, simple copy
         x++ [0] = c[0];
      } else { // we have a $
         b = c + 1;
         if (c[1] == '(') b++;
         if (c[1] == '{') b++;
         if (b[0] == '$')
            e = b + 1;
         else
            for (e = b; isalnum(e[0]) || e[0] == '_'; e++)
               ;
         buff[0] = 0;
         strncat(buff, b, e - b);
         p = getenv(buff);
         if (!p) { // too bad, try UPPER case
            for (t = buff; (t[0] = toupper(t[0])); t++)
               ;
            p = getenv(buff);
         }
         if (!p) { // too bad, try Lower case
            for (t = buff; (t[0] = tolower(t[0])); t++)
               ;
            p = getenv(buff);
         }
         if (!p && !strcmp(buff, "cwd")) { // it is $cwd
            std::string wd = WorkingDirectory();
            strlcpy(buff, wd.c_str(), kBufSize);
            p = buff;
         }
         if (!p && !strcmp(buff, "$")) { // it is $$ (replace by getpid())
            snprintf(buff, kBufSize * 4, "%d", (int)getpid());
            p = buff;
         }
         if (!p) { // too bad, nothing can help
            ier++;
            x++ [0] = c[0];
         } else { // It is OK, copy result
            int lp = strlen(p);
            if (lp >= kBufSize) {
               // make sure lx will be >= kBufSize (see below)
               strlcpy(x, p, kBufSize);
               x += kBufSize;
               break;
            }
            strcpy(x, p);
            x += lp;
            c = (b == c + 1) ? e - 1 : e;
   }
   x[0] = 0;
   lx = x - out;
   if (ier && iter < 3) {
      strlcpy(inp, out, kBufSize);
      goto again;
   ncopy = (lx >= kBufSize) ? kBufSize - 1 : lx;
   xname[0] = 0;
   strncat(xname, out, ncopy);
   if (ier || ncopy != lx) {
      Error(ErrFatal, kErrFatal, "RootdExpandFileName: fatal error:\n\t input: %s\n\t output: %s", fname, xname);
      return true;

   return false;
}

////////////////////////////////////////////////////////////////////////////////
/// Expand a pathname getting rid of special shell characters like ~.$, etc.
/// Returned string must be freed by caller.

char *RootdExpandPathName(const char *name)
{
   const char *patbuf = name;

   // skip leading blanks
   while (*patbuf == ' ') patbuf++;

   // any shell meta characters?
   bool needesc = false;
   for (const char *p = patbuf; *p; p++)
      if (strchr(shellMeta, *p)) {
         needesc = true;
         break;
      }

   // Escape meta characters, if required
   if (needesc) {
      const int kBufSize = kMAXPATHLEN;
      char xname[kBufSize];
      if (RootdExpandFileName(name, xname, kBufSize)) {
         Error(ErrFatal, kErrFatal, "RootdExpandPathName: problem escaping meta characters");
      } else {
         return strdup(xname);
   return strdup(name);