CodeSOD: A Little History

Source control history can go a long way to telling a story. Take Cassi, who needed to run some PHP software which depended on a few binaries and shell calls to do its job. We can see the initial attempt to locate the path of a binary:

function findPathOf($path)
{
    if (file_exists("/usr/bin/$path")) return "/usr/bin/$path";
    return "/usr/local/bin/$path";
}

Now, this worked, so long as the binary was in one of those two places, but in any other case, that's a problem. So someone changed it to:

function findPathOf($path) {
     exec("which " . escapeshellarg($path), $output, $returnVar);
     if ($output != 0) {
         return null;
     }
     return $output[0];
}

This version is completely wrong. $returnVar would contain the shell return code, which would be a non-zero value in the case of an error. $output will always be an array, and even in PHP, an empty array is never going to equal zero. So this didn't work at all. So someone else fixed it.

 function findPathOf($path) {
    if (file_exists("/usr/bin/$path")) return "/usr/bin/$path";
    if (file_exists("/usr/local/bin/$path")) return "/usr/local/bin/$path";
    exec("which " . escapeshellarg($path), $output, $returnVar);
    if ($output != 0) {
        return null;
    }
    return $output[0];
}

This highlights the value of source control history, as this allowed the developer to just do… both. One which is right in only two cases, and one which is never right. Having successfully solved none of the problems with the original solution, someone else decided to give it their best shot.

function findPathOf($path) {
     static $knownPaths = array();

     if ($knownPaths[$path]) return($knownPaths[$path]);
     if (file_exists("/usr/bin/$path")) return $knownPaths[$path] = "/usr/bin/$path";
     if (file_exists("/usr/local/bin/$path")) return $knownPaths[$path] = "/usr/local/bin/$path";
     exec("which " . escapeshellarg($path), $output, $returnVar);
     if ($output != 0) {
         return null;
     }
     $knownPaths[$path] = $output[0];;
     return $output[0];
 }

This version memoizes the search path, ensuring that if we've found the file before, we don't need to find it again. This is a handy optimization, but still fails to address the fact that this code doesn't work except in the specific cases where the file being searched for is in /usr/bin or /usr/local/bin.

Which, at this point, it's likely worth noting, they're not at any point searching for a path, despite the $path variable name, they're searching for an executable in a path, but honestly, is it actually worth noting? Probably not.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *