#include "__call.h" #include "Exception.h" #include "MString.h" #include #include #include #include #include #include #include #include namespace jet { __call::__call(coreutils::ZString &in, coreutils::MString &parentOut, Global &global, Tag *parent, Tag *local) : Tag(in, parentOut, global, parent, local) { if(hasContainer && keywordDefined("input")) throw coreutils::Exception("call tag cannot have both input keyword and have a container."); if(!keywordDefined("pgm")) throw coreutils::Exception("pgm keyword must be specified."); for(ix = 0; ix < 50; ++ix) argv[ix] = NULL; // Reserve up front - argStrings must never reallocate once a // c_str() pointer from one of its elements has been stored into // argv[] below, or that pointer dangles exactly like the bug this // replaces (see the .h's own comment). 50 arg slots + argv[0]'s // own program name. argStrings.reserve(51); argStrings.push_back(resolveKeyword("pgm")); // TODO: Need to peel off the program name only and pass as argv[0]. argv[0] = argStrings.back().c_str(); for(ix = 1; ix < 50; ++ix) { coreutils::MString arg("arg"); arg << ix; if(keywordDefined(arg)) { argStrings.push_back(resolveKeyword(arg)); argv[ix] = argStrings.back().c_str(); } else break; } // pipe()/fork() both used to go unchecked here - on failure (most // realistically pipe() hitting the process's open-file-descriptor // limit, see below) fdo[] is left holding whatever garbage was // already on the stack/in this member array, and every use of it // after this point (close(), read(), and - for a failed fork() - // waitpid(-1, ...), which has the special POSIX meaning "wait for // ANY child" rather than "wait for none") operates on that garbage // or on an unrelated process instead of safely no-oping. That's a // real, confirmed mechanism for exactly the kind of sporadic // memory corruption this codebase has seen: closing/reading a // stale fd that coincidentally matches another subsystem's live // socket, or reaping a concurrent request's own child out from // under it. if(pipe(fdo) != 0) throw coreutils::Exception("call: pipe() failed - " + std::string(strerror(errno)), __FILE__, __LINE__, 500); pid = fork(); if(pid < 0) { close(fdo[0]); close(fdo[1]); throw coreutils::Exception("call: fork() failed - " + std::string(strerror(errno)), __FILE__, __LINE__, 500); } if(pid == 0) { close(fdo[0]); dup2(fdo[1], 1); if(hasContainer) { if(pipe(fdi) == 0) { pid_t innerPid = fork(); if(innerPid == 0) { close(fdi[0]); write(fdi[1], container.getData(), container.getLength()); close(fdi[1]); _exit(0); } else if(innerPid > 0) { close(fdi[1]); dup2(fdi[0], 0); close(fdi[0]); } else { close(fdi[0]); close(fdi[1]); } } } else if(keywordDefined("input")) { coreutils::MString input(resolveKeyword("input")); if(pipe(fdi) == 0) { pid_t innerPid = fork(); if(innerPid == 0) { close(fdi[0]); write(fdi[1], input.getData(), input.getLength()); close(fdi[1]); _exit(0); } else if(innerPid > 0) { close(fdi[1]); dup2(fdi[0], 0); close(fdi[0]); } else { close(fdi[0]); close(fdi[1]); } } } rc = execvpe(argv[0], argv, global.envp); close(fdo[1]); _exit(errno); } close(fdo[1]); if(keywordDefined("name")) { coreutils::MString value; value.read(fdo[0]); storeVariable(resolveKeyword("name"), value, resolveKeyword("scope")); } else out.read(fdo[0]); // fdo[0] (the read end, still open in this - the parent - process) // was never closed here before, leaking one file descriptor on // every single invocation for the life of the process. // Harmless-looking for an occasional , but is used // throughout this codebase (jq/curl parsing, mailcreate, this // session's own per-line-item JS-escaping on the invoice editor, // which alone can issue a double-digit number of calls per page // load) - repeated, routine use was slowly accumulating leaked // fds for as long as this process stayed up, eventually hitting // the real open-files limit and turning into exactly the pipe() // failures (and the undefined behavior above) this fix addresses. close(fdo[0]); waitpid(pid, &status, 0); if(keywordDefined("error")) { global.variables[resolveKeyword("error")] = (status >> 8 & 255); } } }