Say it with me everyone, cleverness is the mother of regret. Rachelbythebay with yet another tale of turn-by-turn navigation down the road to ruin in Fix the unit test and open a giant hole everywhere

Like most dumpster fires, it starts with a mundane business problem:

Our program was going down a road where it might need to create a series of paths. This is where you have “/my/path” and you need a couple of directories nested underneath it, so you end up with “/my/path/project/staging” and “/my/path/project/prod” and things like that.

…that someone solves in a clever way

What I found was… disturbing. I found a function that claimed to create directories, but it wasn’t just a simple passthrough to the usual C library mkdir() function…it was kicking off a subprocess to run the mkdir program

void create_a_directory(path) {
  system("mkdir -p " + path);
}

Unfortunately, in doing this, it created an enormous security hole, too. The C system() call runs subcommands *through a shell*, and whatever you pass to the shell is subject to all kinds of fun shell expansion rules…These shell expansion rules include a semicolon to split commands. This means if you can get a semicolon and another command in there, it will happily run it right after the “mkdir -p /whatever”…It’s trivial to change it to do something far nastier at that point, like opening a shell to somewhere else, exfiltrating data, or whatever. Your program is now wide open to this kind of attack and you’ve changed nothing.

And now for the best part, why it happened at all

So finally, you’re probably wondering why this happened, and why someone would change a function that called a C library function into something that ran a $(*^&$( subprocess. Ah, well, that’s easy. Someone had a unit test that called that directory creation function with a complicated path that had several elements. Sometimes, not all of the intermediate directories existed, and then it would fail. They knew that “mkdir -p” is what they’d do by hand, but they needed the program to do it for them. They changed the common library, checked it in, reran their unit test, and now it started passing, so they were done.

To paraphrase Homer Simpson: To unit tests! The cause of, and solution to, all of dev’s problems