Vulnerability: CRLF injection vulnerability in the LightProcess protocol implementation in hphp/util/light-process.cpp in Facebook HipHop Virtual Machine (HHVM) before 2.4.2 allows remote attackers to execute arbitrary commands by entering a \n (newline) character before the end of a string.
Vulnerability ID: CVE-2014-2208
CWE: CWE-94
Severity score: 6.4
Project: https://github.com/facebook/hhvm
Published: 2014-12-28
File: hphp/util/light-process.cpp
Commit message: Fix escaping in LightProcess protocol

The LightProcess mechanism sends newline terminated commands
over a pipe, but didn't escape newlines contained in the payload.

Reviewed By: otto

Differential Revision: D1184482
Commit author: Bert Maher

Vulnerable version (before fix):
 namespace HPHP { /////////////////////////////////////////////////////////////////////////////// // helper functions  static const unsigned int BUFFER_SIZE = 4096; Mutex LightProcess::s_mutex;  static void read_buf(FILE *fin, char *buf) {   if (!fgets(buf, BUFFER_SIZE, fin)) {     buf[0] = '\0';     return;   }   // get rid of '\n'   buf[strlen(buf) - 1] = '\0'; }  static bool send_fd(int afdt_fd, int fd) {   afdt_error_t err;   errno = 0; static void close_fds(const std::vector<int> &fds) {   } }  /////////////////////////////////////////////////////////////////////////////// // shadow process tasks  static void do_popen(FILE *fin, FILE *fout, int afdt_fd) {   char buf[BUFFER_SIZE];   char cwd[BUFFER_SIZE];    if (!fgets(buf, BUFFER_SIZE, fin)) buf[0] = '\0';   bool read_only = (buf[0] == 'r');    read_buf(fin, buf);    std::string old_cwd = Process::GetCurrentDirectory();   read_buf(fin, cwd);   if (old_cwd != cwd) {     if (chdir(cwd)) {       // Ignore chdir failures, because the compiled version might not have the       // directory any more.       Logger::Warning("Light Process failed chdir to %s.", cwd);     }   }    FILE *f = buf[0] ? ::popen(buf, read_only ? "r" : "w") : nullptr;    if (old_cwd != cwd && chdir(old_cwd.c_str())) {     // only here if we can't change the cwd back static void do_popen(FILE *fin, FILE *fout, int afdt_fd) {   if (f == nullptr) {     Logger::Error("Light process failed popen: %d (%s).", errno,                   folly::errnoStr(errno).c_str());     fprintf(fout, "error\n");     fflush(fout);   } else {     fprintf(fout, "success\n%" PRId64 "\n", (int64_t)f);     fflush(fout);     int fd = fileno(f);     send_fd(afdt_fd, fd);   } }  static void do_pclose(FILE *fin, FILE *fout) {   char buf[BUFFER_SIZE];    int64_t fptr = 0;   read_buf(fin, buf);   sscanf(buf, "%" PRId64, &fptr);   FILE *f = (FILE *)fptr;   int ret = ::pclose(f);    fprintf(fout, "%d\n", ret);   if (ret < 0) {     fprintf(fout, "%d\n", errno);   }   fflush(fout); }  static void do_proc_open(FILE *fin, FILE *fout, int afdt_fd) {   char cmd[BUFFER_SIZE];   read_buf(fin, cmd);   if (strlen(cmd) == 0) {     fprintf(fout, "error\n%d\n", ENOENT);     fflush(fout);     return;   }    char cwd[BUFFER_SIZE];   read_buf(fin, cwd);    char buf[BUFFER_SIZE];   int env_size = 0;   std::vector<std::string> env;   read_buf(fin, buf);   sscanf(buf, "%d", &env_size);   for (int i = 0; i < env_size; i++) {     read_buf(fin, buf);     env.push_back(buf);   }    int pipe_size = 0;   read_buf(fin, buf);   sscanf(buf, "%d", &pipe_size);   std::vector<int> pvals;   for (int i = 0; i < pipe_size; i++) {     int fd_value;     read_buf(fin, buf);     sscanf(buf, "%d", &fd_value);     pvals.push_back(fd_value);   }    std::vector<int> pkeys;   for (int i = 0; i < pipe_size; i++) {     int fd = recv_fd(afdt_fd);     if (fd < 0) {       fprintf(fout, "error\n%d\n", EPROTO);       fflush(fout);       close_fds(pkeys);       return;     }     pkeys.push_back(fd);   }    // now ready to start the child process   pid_t child = fork();   if (child == 0) {     for (int i = 0; i < pipe_size; i++) {       dup2(pkeys[i], pvals[i]);     }     if (strlen(cwd) > 0 && chdir(cwd)) {       // non-zero for error       // chdir failed, the working directory remains unchanged     }     if (!env.empty()) {       char **envp = build_envp(env);       execle("/bin/sh", "sh", "-c", cmd, nullptr, envp);       free(envp);     } else {       execl("/bin/sh", "sh", "-c", cmd, nullptr);     }     _exit(127);   } else if (child > 0) {     // successfully created the child process     fprintf(fout, "%" PRId64 "\n", (int64_t)child);     fflush(fout);   } else {     // failed creating the child process     fprintf(fout, "error\n%d\n", errno);     fflush(fout);   }  static void kill_handler(int sig) { }  static void do_waitpid(FILE *fin, FILE *fout) {   char buf[BUFFER_SIZE];   read_buf(fin, buf);   int64_t p = -1;   int options = 0;   int timeout = 0;   sscanf(buf, "%" PRId64 " %d %d", &p, &options, &timeout);   pid_t pid = (pid_t)p;   int stat;   if (timeout > 0) {     waited = pid;     signal(SIGALRM, kill_handler);     alarm(timeout);   }   pid_t ret = ::waitpid(pid, &stat, options);   alarm(0); // cancel the previous alarm if not triggered yet   waited = 0;   fprintf(fout, "%" PRId64 " %d\n", (int64_t)ret, stat);   if (ret < 0) {     fprintf(fout, "%d\n", errno);   }   fflush(fout); }  static void do_change_user(FILE *fin, FILE *fout) {   char uname[BUFFER_SIZE];   read_buf(fin, uname);   if (strlen(uname) > 0) {     struct passwd *pw = getpwnam(uname);     if (pw) {       if (pw->pw_gid) {         setgid(pw->pw_gid); void LightProcess::Close() { void LightProcess::closeShadow() {   Lock lock(m_procMutex);   if (m_shadowProcess) {     fprintf(m_fout, "exit\n");     fflush(m_fout);     fclose(m_fin);     fclose(m_fout); void LightProcess::runShadow(int fdin, int fdout) {   FILE *fin = fdopen(fdin, "r");   FILE *fout = fdopen(fdout, "w");    char buf[BUFFER_SIZE];    pollfd pfd[1];   pfd[0].fd = fdin; void LightProcess::runShadow(int fdin, int fdout) {       continue;     }     if (pfd[0].revents & POLLIN) {       if (!fgets(buf, BUFFER_SIZE, fin)) buf[0] = '\0';       if (strncmp(buf, "exit", 4) == 0) {         Logger::Info("LightProcess exiting upon request");         break;       } else if (strncmp(buf, "popen", 5) == 0) {         do_popen(fin, fout, m_afdt_fd);       } else if (strncmp(buf, "pclose", 6) == 0) {         do_pclose(fin, fout);       } else if (strncmp(buf, "proc_open", 9) == 0) {         do_proc_open(fin, fout, m_afdt_fd);       } else if (strncmp(buf, "waitpid", 7) == 0) {         do_waitpid(fin, fout);       } else if (strncmp(buf, "change_user", 11) == 0) {         do_change_user(fin, fout);       } else if (buf[0]) {         Logger::Info("LightProcess got invalid command: %.20s", buf);       }     } else if (pfd[0].revents & POLLHUP) {       // no more command can come in FILE *LightProcess::LightPopenImpl(const char *cmd, const char *type,   int id = GetId();   Lock lock(g_procs[id].m_procMutex);    fprintf(g_procs[id].m_fout, "popen\n%s\n%s\n%s\n", type, cmd, cwd);   fflush(g_procs[id].m_fout);    char buf[BUFFER_SIZE];   read_buf(g_procs[id].m_fin, buf);   if (strncmp(buf, "error", 5) == 0) {     return nullptr;   }    int64_t fptr = 0;   read_buf(g_procs[id].m_fin, buf);   sscanf(buf, "%" PRId64, &fptr);   if (!fptr) {     Logger::Error("Light process failed to return the file pointer.");     return nullptr; int LightProcess::pclose(FILE *f) {   int64_t f2 = it->second;   g_procs[id].m_popenMap.erase((int64_t)f);   fclose(f);   fprintf(g_procs[id].m_fout, "pclose\n%" PRId64 "\n", f2);   fflush(g_procs[id].m_fout);    char buf[BUFFER_SIZE];   read_buf(g_procs[id].m_fin, buf);   int ret = -1;   sscanf(buf, "%d", &ret);   if (ret < 0) {     read_buf(g_procs[id].m_fin, buf);     sscanf(buf, "%d", &errno);   }   return ret; } pid_t LightProcess::proc_open(const char *cmd, const std::vector<int> &created,   always_assert(Available());   always_assert(created.size() == desired.size());    if (fprintf(g_procs[id].m_fout, "proc_open\n%s\n%s\n", cmd, cwd) <= 0) {     Logger::Error("Failed to send command proc_open");     return -1;   }   fprintf(g_procs[id].m_fout, "%d\n", (int)env.size());   for (unsigned int i = 0; i < env.size(); i++) {     fprintf(g_procs[id].m_fout, "%s\n", env[i].c_str());   }    fprintf(g_procs[id].m_fout, "%d\n", (int)created.size());    for (unsigned int i = 0; i < desired.size(); i++) {     fprintf(g_procs[id].m_fout, "%d\n", desired[i]);   }   fflush(g_procs[id].m_fout);   bool error_send = false;   int save_errno = 0;   for (unsigned int i = 0; i < created.size(); i++) { pid_t LightProcess::proc_open(const char *cmd, const std::vector<int> &created,     }   }    char buf[BUFFER_SIZE];   read_buf(g_procs[id].m_fin, buf);   if (strncmp(buf, "error", 5) == 0) {     read_buf(g_procs[id].m_fin, buf);     sscanf(buf, "%d", &errno);     if (error_send) {       // On this error, the receiver side returns dummy errno,       // use the sender side errno here.       errno = save_errno;     }     return -1;   }   int64_t pid = -1;   sscanf(buf, "%" PRId64, &pid);   assert(pid);   return (pid_t)pid; }  pid_t LightProcess::waitpid(pid_t pid, int *stat_loc, int options,   int id = GetId();   Lock lock(g_procs[id].m_procMutex);    fprintf(g_procs[id].m_fout, "waitpid\n%" PRId64 " %d %d\n", (int64_t)pid, options,           timeout);   fflush(g_procs[id].m_fout);    char buf[BUFFER_SIZE];   read_buf(g_procs[id].m_fin, buf);   if (!buf[0]) return -1;   int64_t ret;   int stat;   sscanf(buf, "%" PRId64 " %d", &ret, &stat);   *stat_loc = stat;   if (ret < 0) {     read_buf(g_procs[id].m_fin, buf);     sscanf(buf, "%d", &errno);   }   return (pid_t)ret; } void LightProcess::ChangeUser(const std::string &username) {   if (username.empty()) return;   for (int i = 0; i < g_procsCount; i++) {     Lock lock(g_procs[i].m_procMutex);     fprintf(g_procs[i].m_fout, "change_user\n%s\n", username.c_str());     fflush(g_procs[i].m_fout);   } } 

Fixed version (after fix):
 namespace HPHP { /////////////////////////////////////////////////////////////////////////////// // helper functions  Mutex LightProcess::s_mutex;  static bool send_fd(int afdt_fd, int fd) {   afdt_error_t err;   errno = 0; static void close_fds(const std::vector<int> &fds) {   } }  static void lwp_write(FILE *fout, const std::string &buf) {   size_t len = buf.length();   fwrite(&len, sizeof(len), 1, fout);   fwrite(buf.c_str(), sizeof(buf[0]), len, fout);   fflush(fout); }  static void lwp_write_int32(FILE *fout, int32_t d) {   fwrite(&d, sizeof(d), 1, fout);   fflush(fout); }  static void lwp_write_int64(FILE *fout, int64_t d) {   fwrite(&d, sizeof(d), 1, fout);   fflush(fout); }  static void lwp_read(FILE *fin, std::string &buf) {   size_t len;   fread(&len, sizeof(len), 1, fin);   char *buffer = (char *)malloc(len + 1);   fread(buffer, sizeof(*buffer), len, fin);   buffer[len] = '\0';   buf = std::string(buffer);   free(buffer); }  static void lwp_read_int32(FILE *fin, int32_t &d) {   fread(&d, sizeof(d), 1, fin); }  static void lwp_read_int64(FILE *fin, int64_t &d) {   fread(&d, sizeof(d), 1, fin); }  /////////////////////////////////////////////////////////////////////////////// // shadow process tasks  static void do_popen(FILE *fin, FILE *fout, int afdt_fd) {   std::string buf;   std::string cwd;    lwp_read(fin, buf);   bool read_only = (buf[0] == 'r');    lwp_read(fin, buf);    std::string old_cwd = Process::GetCurrentDirectory();   lwp_read(fin, cwd);    if (old_cwd != cwd) {     if (chdir(cwd.c_str())) {       // Ignore chdir failures, because the compiled version might not have the       // directory any more.       Logger::Warning("Light Process failed chdir to %s.", cwd.c_str());     }   }    FILE *f = buf[0] ? ::popen(buf.c_str(), read_only ? "r" : "w") : nullptr;    if (old_cwd != cwd && chdir(old_cwd.c_str())) {     // only here if we can't change the cwd back static void do_popen(FILE *fin, FILE *fout, int afdt_fd) {   if (f == nullptr) {     Logger::Error("Light process failed popen: %d (%s).", errno,                   folly::errnoStr(errno).c_str());     lwp_write(fout, "error");   } else {     lwp_write(fout, "success");     lwp_write_int64(fout, (int64_t)f);     int fd = fileno(f);     send_fd(afdt_fd, fd);   } }  static void do_pclose(FILE *fin, FILE *fout) {   int64_t fptr = 0;   lwp_read_int64(fin, fptr);   FILE *f = (FILE *)fptr;   int ret = ::pclose(f);    lwp_write_int32(fout, ret);   if (ret < 0) {     lwp_write_int32(fout, errno);   }   fflush(fout); }  static void do_proc_open(FILE *fin, FILE *fout, int afdt_fd) {   std::string cmd;   lwp_read(fin, cmd);    std::string cwd;   lwp_read(fin, cwd);    std::string buf;   int env_size = 0;   std::vector<std::string> env;   lwp_read_int32(fin, env_size);   for (int i = 0; i < env_size; i++) {     lwp_read(fin, buf);     env.push_back(buf);   }    int pipe_size = 0;   lwp_read_int32(fin, pipe_size);   std::vector<int> pvals;   for (int i = 0; i < pipe_size; i++) {     int fd_value;     lwp_read_int32(fin, fd_value);     pvals.push_back(fd_value);   }    std::vector<int> pkeys;   for (int i = 0; i < pipe_size; i++) {     int fd = recv_fd(afdt_fd);     if (fd < 0) {       lwp_write(fout, "error");       lwp_write_int32(fout, EPROTO);       fflush(fout);       close_fds(pkeys);       return;     }     pkeys.push_back(fd);   }    // indicate error if an empty command was received   if (cmd.length() == 0) {     lwp_write(fout, "error");     lwp_write_int32(fout, ENOENT);     return;   }    // now ready to start the child process   pid_t child = fork();   if (child == 0) {     for (int i = 0; i < pipe_size; i++) {       dup2(pkeys[i], pvals[i]);     }     if (cwd.length() > 0 && chdir(cwd.c_str())) {       // non-zero for error       // chdir failed, the working directory remains unchanged     }     if (!env.empty()) {       char **envp = build_envp(env);       execle("/bin/sh", "sh", "-c", cmd.c_str(), nullptr, envp);       free(envp);     } else {       execl("/bin/sh", "sh", "-c", cmd.c_str(), nullptr);     }     _exit(127);   } else if (child > 0) {     // successfully created the child process     lwp_write(fout, "success");     lwp_write_int64(fout, (int64_t)child);     fflush(fout);   } else {     // failed creating the child process     lwp_write(fout, "error");     lwp_write_int32(fout, errno);     fflush(fout);   }  static void kill_handler(int sig) { }  static void do_waitpid(FILE *fin, FILE *fout) {   int64_t p = -1;   int options = 0;   int timeout = 0;   lwp_read_int64(fin, p);   lwp_read_int32(fin, options);   lwp_read_int32(fin, timeout);    pid_t pid = (pid_t)p;   int stat;   if (timeout > 0) {     waited = pid;     signal(SIGALRM, kill_handler);     alarm(timeout);   }    pid_t ret = ::waitpid(pid, &stat, options);   alarm(0); // cancel the previous alarm if not triggered yet   waited = 0;   lwp_write_int64(fout, ret);   lwp_write_int32(fout, stat);   if (ret < 0) {     lwp_write_int32(fout, errno);   }   fflush(fout); }  static void do_change_user(FILE *fin, FILE *fout) {   std::string uname;   lwp_read(fin, uname);   if (uname.length() > 0) {     struct passwd *pw = getpwnam(uname.c_str());     if (pw) {       if (pw->pw_gid) {         setgid(pw->pw_gid); void LightProcess::Close() { void LightProcess::closeShadow() {   Lock lock(m_procMutex);   if (m_shadowProcess) {     lwp_write(m_fout, "exit");     fflush(m_fout);     fclose(m_fin);     fclose(m_fout); void LightProcess::runShadow(int fdin, int fdout) {   FILE *fin = fdopen(fdin, "r");   FILE *fout = fdopen(fdout, "w");    std::string buf;    pollfd pfd[1];   pfd[0].fd = fdin; void LightProcess::runShadow(int fdin, int fdout) {       continue;     }     if (pfd[0].revents & POLLIN) {       lwp_read(fin, buf);       if (buf == "exit") {         Logger::Info("LightProcess exiting upon request");         break;       } else if (buf == "popen") {         do_popen(fin, fout, m_afdt_fd);       } else if (buf == "pclose") {         do_pclose(fin, fout);       } else if (buf == "proc_open") {         do_proc_open(fin, fout, m_afdt_fd);       } else if (buf == "waitpid") {         do_waitpid(fin, fout);       } else if (buf == "change_user") {         do_change_user(fin, fout);       } else if (buf[0]) {         Logger::Info("LightProcess got invalid command: %.20s", buf.c_str());       }     } else if (pfd[0].revents & POLLHUP) {       // no more command can come in FILE *LightProcess::LightPopenImpl(const char *cmd, const char *type,   int id = GetId();   Lock lock(g_procs[id].m_procMutex);    FILE *fout = g_procs[id].m_fout;   lwp_write(fout, "popen");   lwp_write(fout, type);   lwp_write(fout, cmd);   lwp_write(fout, cwd ? cwd : "");   fflush(fout);    std::string buf;   FILE *fin = g_procs[id].m_fin;   lwp_read(fin, buf);   if (buf == "error") {     return nullptr;   }    int64_t fptr = 0;   lwp_read_int64(fin, fptr);   if (!fptr) {     Logger::Error("Light process failed to return the file pointer.");     return nullptr; int LightProcess::pclose(FILE *f) {   int64_t f2 = it->second;   g_procs[id].m_popenMap.erase((int64_t)f);   fclose(f);    lwp_write(g_procs[id].m_fout, "pclose");   lwp_write_int64(g_procs[id].m_fout, f2);    int ret = -1;   lwp_read_int32(g_procs[id].m_fin, ret);   if (ret < 0) {     lwp_read_int32(g_procs[id].m_fin, errno);   }   return ret; } pid_t LightProcess::proc_open(const char *cmd, const std::vector<int> &created,   always_assert(Available());   always_assert(created.size() == desired.size());    FILE *fout = g_procs[id].m_fout;   lwp_write(fout, "proc_open");   lwp_write(fout, cmd);   lwp_write(fout, cwd ? cwd : "");   lwp_write_int32(fout, (int)env.size());   for (unsigned int i = 0; i < env.size(); i++) {     lwp_write(fout, env[i]);   }    lwp_write_int32(fout, (int)created.size());   for (unsigned int i = 0; i < desired.size(); i++) {     lwp_write_int32(fout, desired[i]);   }   fflush(fout);    bool error_send = false;   int save_errno = 0;   for (unsigned int i = 0; i < created.size(); i++) { pid_t LightProcess::proc_open(const char *cmd, const std::vector<int> &created,     }   }    std::string buf;   FILE *fin = g_procs[id].m_fin;   lwp_read(fin, buf);   if (buf == "error") {     lwp_read_int32(fin, errno);     if (error_send) {       // On this error, the receiver side returns dummy errno,       // use the sender side errno here.       errno = save_errno;     }     return -1;   }   always_assert(buf == "success");   int64_t pid = -1;   lwp_read_int64(fin, pid);   always_assert(pid);   return (pid_t)pid; }  pid_t LightProcess::waitpid(pid_t pid, int *stat_loc, int options,   int id = GetId();   Lock lock(g_procs[id].m_procMutex);    FILE *fout = g_procs[id].m_fout;   lwp_write(fout, "waitpid");   lwp_write_int64(fout, (int64_t)pid);   lwp_write_int32(fout, options);   lwp_write_int32(fout, timeout);   fflush(g_procs[id].m_fout);    int64_t ret;   int stat;   FILE *fin = g_procs[id].m_fin;   lwp_read_int64(fin, ret);   lwp_read_int32(fin, stat);    *stat_loc = stat;   if (ret < 0) {     lwp_read_int32(fin, errno);   }   return (pid_t)ret; } void LightProcess::ChangeUser(const std::string &username) {   if (username.empty()) return;   for (int i = 0; i < g_procsCount; i++) {     Lock lock(g_procs[i].m_procMutex);     FILE *fout = g_procs[i].m_fout;     lwp_write(fout, "change_user");     lwp_write(fout, username);     fflush(fout);   } } 

Patch:
patch: @@ -42,18 +42,8 @@ namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 // helper functions
 
-static const unsigned int BUFFER_SIZE = 4096;
 Mutex LightProcess::s_mutex;
 
-static void read_buf(FILE *fin, char *buf) {
-  if (!fgets(buf, BUFFER_SIZE, fin)) {
-    buf[0] = '\0';
-    return;
-  }
-  // get rid of '\n'
-  buf[strlen(buf) - 1] = '\0';
-}
-
 static bool send_fd(int afdt_fd, int fd) {
   afdt_error_t err;
   errno = 0;
@@ -101,29 +91,65 @@ static void close_fds(const std::vector<int> &fds) {
   }
 }
 
+static void lwp_write(FILE *fout, const std::string &buf) {
+  size_t len = buf.length();
+  fwrite(&len, sizeof(len), 1, fout);
+  fwrite(buf.c_str(), sizeof(buf[0]), len, fout);
+  fflush(fout);
+}
+
+static void lwp_write_int32(FILE *fout, int32_t d) {
+  fwrite(&d, sizeof(d), 1, fout);
+  fflush(fout);
+}
+
+static void lwp_write_int64(FILE *fout, int64_t d) {
+  fwrite(&d, sizeof(d), 1, fout);
+  fflush(fout);
+}
+
+static void lwp_read(FILE *fin, std::string &buf) {
+  size_t len;
+  fread(&len, sizeof(len), 1, fin);
+  char *buffer = (char *)malloc(len + 1);
+  fread(buffer, sizeof(*buffer), len, fin);
+  buffer[len] = '\0';
+  buf = std::string(buffer);
+  free(buffer);
+}
+
+static void lwp_read_int32(FILE *fin, int32_t &d) {
+  fread(&d, sizeof(d), 1, fin);
+}
+
+static void lwp_read_int64(FILE *fin, int64_t &d) {
+  fread(&d, sizeof(d), 1, fin);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // shadow process tasks
 
 static void do_popen(FILE *fin, FILE *fout, int afdt_fd) {
-  char buf[BUFFER_SIZE];
-  char cwd[BUFFER_SIZE];
+  std::string buf;
+  std::string cwd;
 
-  if (!fgets(buf, BUFFER_SIZE, fin)) buf[0] = '\0';
+  lwp_read(fin, buf);
   bool read_only = (buf[0] == 'r');
 
-  read_buf(fin, buf);
+  lwp_read(fin, buf);
 
   std::string old_cwd = Process::GetCurrentDirectory();
-  read_buf(fin, cwd);
+  lwp_read(fin, cwd);
+
   if (old_cwd != cwd) {
-    if (chdir(cwd)) {
+    if (chdir(cwd.c_str())) {
       // Ignore chdir failures, because the compiled version might not have the
       // directory any more.
-      Logger::Warning("Light Process failed chdir to %s.", cwd);
+      Logger::Warning("Light Process failed chdir to %s.", cwd.c_str());
     }
   }
 
-  FILE *f = buf[0] ? ::popen(buf, read_only ? "r" : "w") : nullptr;
+  FILE *f = buf[0] ? ::popen(buf.c_str(), read_only ? "r" : "w") : nullptr;
 
   if (old_cwd != cwd && chdir(old_cwd.c_str())) {
     // only here if we can't change the cwd back
@@ -132,102 +158,100 @@ static void do_popen(FILE *fin, FILE *fout, int afdt_fd) {
   if (f == nullptr) {
     Logger::Error("Light process failed popen: %d (%s).", errno,
                   folly::errnoStr(errno).c_str());
-    fprintf(fout, "error\n");
-    fflush(fout);
+    lwp_write(fout, "error");
   } else {
-    fprintf(fout, "success\n%" PRId64 "\n", (int64_t)f);
-    fflush(fout);
+    lwp_write(fout, "success");
+    lwp_write_int64(fout, (int64_t)f);
     int fd = fileno(f);
     send_fd(afdt_fd, fd);
   }
 }
 
 static void do_pclose(FILE *fin, FILE *fout) {
-  char buf[BUFFER_SIZE];
-
   int64_t fptr = 0;
-  read_buf(fin, buf);
-  sscanf(buf, "%" PRId64, &fptr);
+  lwp_read_int64(fin, fptr);
   FILE *f = (FILE *)fptr;
   int ret = ::pclose(f);
 
-  fprintf(fout, "%d\n", ret);
+  lwp_write_int32(fout, ret);
   if (ret < 0) {
-    fprintf(fout, "%d\n", errno);
+    lwp_write_int32(fout, errno);
   }
   fflush(fout);
 }
 
 static void do_proc_open(FILE *fin, FILE *fout, int afdt_fd) {
-  char cmd[BUFFER_SIZE];
-  read_buf(fin, cmd);
-  if (strlen(cmd) == 0) {
-    fprintf(fout, "error\n%d\n", ENOENT);
-    fflush(fout);
-    return;
-  }
+  std::string cmd;
+  lwp_read(fin, cmd);
 
-  char cwd[BUFFER_SIZE];
-  read_buf(fin, cwd);
+  std::string cwd;
+  lwp_read(fin, cwd);
 
-  char buf[BUFFER_SIZE];
+  std::string buf;
   int env_size = 0;
   std::vector<std::string> env;
-  read_buf(fin, buf);
-  sscanf(buf, "%d", &env_size);
+  lwp_read_int32(fin, env_size);
   for (int i = 0; i < env_size; i++) {
-    read_buf(fin, buf);
+    lwp_read(fin, buf);
     env.push_back(buf);
   }
 
   int pipe_size = 0;
-  read_buf(fin, buf);
-  sscanf(buf, "%d", &pipe_size);
+  lwp_read_int32(fin, pipe_size);
   std::vector<int> pvals;
   for (int i = 0; i < pipe_size; i++) {
     int fd_value;
-    read_buf(fin, buf);
-    sscanf(buf, "%d", &fd_value);
+    lwp_read_int32(fin, fd_value);
     pvals.push_back(fd_value);
   }
 
   std::vector<int> pkeys;
   for (int i = 0; i < pipe_size; i++) {
     int fd = recv_fd(afdt_fd);
     if (fd < 0) {
-      fprintf(fout, "error\n%d\n", EPROTO);
+      lwp_write(fout, "error");
+      lwp_write_int32(fout, EPROTO);
       fflush(fout);
       close_fds(pkeys);
       return;
     }
     pkeys.push_back(fd);
   }
 
+  // indicate error if an empty command was received
+  if (cmd.length() == 0) {
+    lwp_write(fout, "error");
+    lwp_write_int32(fout, ENOENT);
+    return;
+  }
+
   // now ready to start the child process
   pid_t child = fork();
   if (child == 0) {
     for (int i = 0; i < pipe_size; i++) {
       dup2(pkeys[i], pvals[i]);
     }
-    if (strlen(cwd) > 0 && chdir(cwd)) {
+    if (cwd.length() > 0 && chdir(cwd.c_str())) {
       // non-zero for error
       // chdir failed, the working directory remains unchanged
     }
     if (!env.empty()) {
       char **envp = build_envp(env);
-      execle("/bin/sh", "sh", "-c", cmd, nullptr, envp);
+      execle("/bin/sh", "sh", "-c", cmd.c_str(), nullptr, envp);
       free(envp);
     } else {
-      execl("/bin/sh", "sh", "-c", cmd, nullptr);
+      execl("/bin/sh", "sh", "-c", cmd.c_str(), nullptr);
     }
     _exit(127);
   } else if (child > 0) {
     // successfully created the child process
-    fprintf(fout, "%" PRId64 "\n", (int64_t)child);
+    lwp_write(fout, "success");
+    lwp_write_int64(fout, (int64_t)child);
     fflush(fout);
   } else {
     // failed creating the child process
-    fprintf(fout, "error\n%d\n", errno);
+    lwp_write(fout, "error");
+    lwp_write_int32(fout, errno);
     fflush(fout);
   }
 
@@ -243,34 +267,37 @@ static void kill_handler(int sig) {
 }
 
 static void do_waitpid(FILE *fin, FILE *fout) {
-  char buf[BUFFER_SIZE];
-  read_buf(fin, buf);
   int64_t p = -1;
   int options = 0;
   int timeout = 0;
-  sscanf(buf, "%" PRId64 " %d %d", &p, &options, &timeout);
+  lwp_read_int64(fin, p);
+  lwp_read_int32(fin, options);
+  lwp_read_int32(fin, timeout);
+
   pid_t pid = (pid_t)p;
   int stat;
   if (timeout > 0) {
     waited = pid;
     signal(SIGALRM, kill_handler);
     alarm(timeout);
   }
+
   pid_t ret = ::waitpid(pid, &stat, options);
   alarm(0); // cancel the previous alarm if not triggered yet
   waited = 0;
-  fprintf(fout, "%" PRId64 " %d\n", (int64_t)ret, stat);
+  lwp_write_int64(fout, ret);
+  lwp_write_int32(fout, stat);
   if (ret < 0) {
-    fprintf(fout, "%d\n", errno);
+    lwp_write_int32(fout, errno);
   }
   fflush(fout);
 }
 
 static void do_change_user(FILE *fin, FILE *fout) {
-  char uname[BUFFER_SIZE];
-  read_buf(fin, uname);
-  if (strlen(uname) > 0) {
-    struct passwd *pw = getpwnam(uname);
+  std::string uname;
+  lwp_read(fin, uname);
+  if (uname.length() > 0) {
+    struct passwd *pw = getpwnam(uname.c_str());
     if (pw) {
       if (pw->pw_gid) {
         setgid(pw->pw_gid);
@@ -448,7 +475,7 @@ void LightProcess::Close() {
 void LightProcess::closeShadow() {
   Lock lock(m_procMutex);
   if (m_shadowProcess) {
-    fprintf(m_fout, "exit\n");
+    lwp_write(m_fout, "exit");
     fflush(m_fout);
     fclose(m_fin);
     fclose(m_fout);
@@ -480,7 +507,7 @@ void LightProcess::runShadow(int fdin, int fdout) {
   FILE *fin = fdopen(fdin, "r");
   FILE *fout = fdopen(fdout, "w");
 
-  char buf[BUFFER_SIZE];
+  std::string buf;
 
   pollfd pfd[1];
   pfd[0].fd = fdin;
@@ -491,22 +518,22 @@ void LightProcess::runShadow(int fdin, int fdout) {
       continue;
     }
     if (pfd[0].revents & POLLIN) {
-      if (!fgets(buf, BUFFER_SIZE, fin)) buf[0] = '\0';
-      if (strncmp(buf, "exit", 4) == 0) {
+      lwp_read(fin, buf);
+      if (buf == "exit") {
         Logger::Info("LightProcess exiting upon request");
         break;
-      } else if (strncmp(buf, "popen", 5) == 0) {
+      } else if (buf == "popen") {
         do_popen(fin, fout, m_afdt_fd);
-      } else if (strncmp(buf, "pclose", 6) == 0) {
+      } else if (buf == "pclose") {
         do_pclose(fin, fout);
-      } else if (strncmp(buf, "proc_open", 9) == 0) {
+      } else if (buf == "proc_open") {
         do_proc_open(fin, fout, m_afdt_fd);
-      } else if (strncmp(buf, "waitpid", 7) == 0) {
+      } else if (buf == "waitpid") {
         do_waitpid(fin, fout);
-      } else if (strncmp(buf, "change_user", 11) == 0) {
+      } else if (buf == "change_user") {
         do_change_user(fin, fout);
       } else if (buf[0]) {
-        Logger::Info("LightProcess got invalid command: %.20s", buf);
+        Logger::Info("LightProcess got invalid command: %.20s", buf.c_str());
       }
     } else if (pfd[0].revents & POLLHUP) {
       // no more command can come in
@@ -566,18 +593,22 @@ FILE *LightProcess::LightPopenImpl(const char *cmd, const char *type,
   int id = GetId();
   Lock lock(g_procs[id].m_procMutex);
 
-  fprintf(g_procs[id].m_fout, "popen\n%s\n%s\n%s\n", type, cmd, cwd);
-  fflush(g_procs[id].m_fout);
+  FILE *fout = g_procs[id].m_fout;
+  lwp_write(fout, "popen");
+  lwp_write(fout, type);
+  lwp_write(fout, cmd);
+  lwp_write(fout, cwd ? cwd : "");
+  fflush(fout);
 
-  char buf[BUFFER_SIZE];
-  read_buf(g_procs[id].m_fin, buf);
-  if (strncmp(buf, "error", 5) == 0) {
+  std::string buf;
+  FILE *fin = g_procs[id].m_fin;
+  lwp_read(fin, buf);
+  if (buf == "error") {
     return nullptr;
   }
 
   int64_t fptr = 0;
-  read_buf(g_procs[id].m_fin, buf);
-  sscanf(buf, "%" PRId64, &fptr);
+  lwp_read_int64(fin, fptr);
   if (!fptr) {
     Logger::Error("Light process failed to return the file pointer.");
     return nullptr;
@@ -611,16 +642,14 @@ int LightProcess::pclose(FILE *f) {
   int64_t f2 = it->second;
   g_procs[id].m_popenMap.erase((int64_t)f);
   fclose(f);
-  fprintf(g_procs[id].m_fout, "pclose\n%" PRId64 "\n", f2);
-  fflush(g_procs[id].m_fout);
 
-  char buf[BUFFER_SIZE];
-  read_buf(g_procs[id].m_fin, buf);
+  lwp_write(g_procs[id].m_fout, "pclose");
+  lwp_write_int64(g_procs[id].m_fout, f2);
+
   int ret = -1;
-  sscanf(buf, "%d", &ret);
+  lwp_read_int32(g_procs[id].m_fin, ret);
   if (ret < 0) {
-    read_buf(g_procs[id].m_fin, buf);
-    sscanf(buf, "%d", &errno);
+    lwp_read_int32(g_procs[id].m_fin, errno);
   }
   return ret;
 }
@@ -634,21 +663,21 @@ pid_t LightProcess::proc_open(const char *cmd, const std::vector<int> &created,
   always_assert(Available());
   always_assert(created.size() == desired.size());
 
-  if (fprintf(g_procs[id].m_fout, "proc_open\n%s\n%s\n", cmd, cwd) <= 0) {
-    Logger::Error("Failed to send command proc_open");
-    return -1;
-  }
-  fprintf(g_procs[id].m_fout, "%d\n", (int)env.size());
+  FILE *fout = g_procs[id].m_fout;
+  lwp_write(fout, "proc_open");
+  lwp_write(fout, cmd);
+  lwp_write(fout, cwd ? cwd : "");
+  lwp_write_int32(fout, (int)env.size());
   for (unsigned int i = 0; i < env.size(); i++) {
-    fprintf(g_procs[id].m_fout, "%s\n", env[i].c_str());
+    lwp_write(fout, env[i]);
   }
 
-  fprintf(g_procs[id].m_fout, "%d\n", (int)created.size());
-
+  lwp_write_int32(fout, (int)created.size());
   for (unsigned int i = 0; i < desired.size(); i++) {
-    fprintf(g_procs[id].m_fout, "%d\n", desired[i]);
+    lwp_write_int32(fout, desired[i]);
   }
-  fflush(g_procs[id].m_fout);
+  fflush(fout);
+
   bool error_send = false;
   int save_errno = 0;
   for (unsigned int i = 0; i < created.size(); i++) {
@@ -659,21 +688,22 @@ pid_t LightProcess::proc_open(const char *cmd, const std::vector<int> &created,
     }
   }
 
-  char buf[BUFFER_SIZE];
-  read_buf(g_procs[id].m_fin, buf);
-  if (strncmp(buf, "error", 5) == 0) {
-    read_buf(g_procs[id].m_fin, buf);
-    sscanf(buf, "%d", &errno);
+  std::string buf;
+  FILE *fin = g_procs[id].m_fin;
+  lwp_read(fin, buf);
+  if (buf == "error") {
+    lwp_read_int32(fin, errno);
     if (error_send) {
       // On this error, the receiver side returns dummy errno,
       // use the sender side errno here.
       errno = save_errno;
     }
     return -1;
   }
+  always_assert(buf == "success");
   int64_t pid = -1;
-  sscanf(buf, "%" PRId64, &pid);
-  assert(pid);
+  lwp_read_int64(fin, pid);
+  always_assert(pid);
   return (pid_t)pid;
 }
 
@@ -687,20 +717,22 @@ pid_t LightProcess::waitpid(pid_t pid, int *stat_loc, int options,
   int id = GetId();
   Lock lock(g_procs[id].m_procMutex);
 
-  fprintf(g_procs[id].m_fout, "waitpid\n%" PRId64 " %d %d\n", (int64_t)pid, options,
-          timeout);
+  FILE *fout = g_procs[id].m_fout;
+  lwp_write(fout, "waitpid");
+  lwp_write_int64(fout, (int64_t)pid);
+  lwp_write_int32(fout, options);
+  lwp_write_int32(fout, timeout);
   fflush(g_procs[id].m_fout);
 
-  char buf[BUFFER_SIZE];
-  read_buf(g_procs[id].m_fin, buf);
-  if (!buf[0]) return -1;
   int64_t ret;
   int stat;
-  sscanf(buf, "%" PRId64 " %d", &ret, &stat);
+  FILE *fin = g_procs[id].m_fin;
+  lwp_read_int64(fin, ret);
+  lwp_read_int32(fin, stat);
+
   *stat_loc = stat;
   if (ret < 0) {
-    read_buf(g_procs[id].m_fin, buf);
-    sscanf(buf, "%d", &errno);
+    lwp_read_int32(fin, errno);
   }
   return (pid_t)ret;
 }
@@ -726,8 +758,10 @@ void LightProcess::ChangeUser(const std::string &username) {
   if (username.empty()) return;
   for (int i = 0; i < g_procsCount; i++) {
     Lock lock(g_procs[i].m_procMutex);
-    fprintf(g_procs[i].m_fout, "change_user\n%s\n", username.c_str());
-    fflush(g_procs[i].m_fout);
+    FILE *fout = g_procs[i].m_fout;
+    lwp_write(fout, "change_user");
+    lwp_write(fout, username);
+    fflush(fout);
   }
 }
path: hphp/util/light-process.cpp