Back to elFinder features

If you are not familiar with the software we are talking about, you only need to know it is nothing more than a file manager for the web. It as features like uploading and downloading files, zipping things, previewing doohickeys and so on.

For that, it exposes a single server entry point, named the connector, with a JSON like API. All API calls include a cmd parameter, with the action to perform, and often one or more target arguments representing the files to manipulate.

The target parameter is of the form source_path, where source is the storage in which the file is, and path the base64 encoded path of the file in the storage. We'll ignore the source part in our case study as it won't play any role. Just consider our source is always the file system root directory.

GET /elfinder/php/connector.minimal.php?cmd=file&target=l1_YWFhLnR4dA&download=1&reqid=17fbb7e3da0cd HTTP/1.1
Host: 192.168.122.113
[...]

For example, in the previous request, the file action (AKA download) is called on the aaa.txt file. Relative paths are taken from the configured exposed directory.

And then back to business

Let's imagine we are in 2010 working on a software that has not been extensively reviewed. Let's say we face a software that handles file paths. To finish, imagine we want to test for path traversal issues. The first instinctive action would be to provide the application with a long ../ chain and to try reaching /etc/passwd that way.

If that does not work we would probably try some bypasses, adding a few slashes, encoding on key characters and so on.

But this should not succeed in our case right? Not on a 6 years old version. Not on a software that has been thoroughly reviewed by talented people.

The fact is we tried.

The fact is it passed.

Down the rabbit hole

We found ourselves in a situation where we had an unexpected vulnerability, and no idea about why it is here and went unnoticed for such a long time. However, if we analyze our payload, what appears is that the application is probably messing up looking for ../ and trying to delete them. This is often seen in beginner CTF web challenges.

Building upon our luck, let's grep for "..".

$ find . -name "*.php" -exec grep -HF ".." {} \;
./elFinderVolumeDriver.class.php:    * System Root path (Unix like: '/', Windows: '\', 'C:\' or 'D:\'...)
./elFinderVolumeDriver.class.php:                           $name = preg_replace('/^(.*?)(\..*)?$/', '$1_'.$files[$name]++.'$2', $name);
./elFinderVolumeDriver.class.php:                           $name != '.' && $name != '..' && $this->rmTmb($this->stat($p));
./elFinderVolumeDriver.class.php:           if (substr($path, 0, 3) === '..' . $separator) {
./elFinderVolumeDriver.class.php:           // normalize `/../`
./elFinderVolumeDriver.class.php:                                   if ($file === '.' || $file === '..') {
./elFinderVolumeDriver.class.php:                           if ($file !== '.' && $file !== '..') {
./elFinderVolumeDriver.class.php:                                                           if ($entry !== "." && $entry !== "..") {
./elFinderVolumeDriver.class.php:    * Ususaly used for images, but can be realize for video etc...
./elFinderVolumeFTP.class.php:              if (count($info) < 9 || $info[8] == '.' || $info[8] == '..') {
./elFinderVolumeFTP.class.php:                      if (($comp != '..')
./elFinderVolumeFTP.class.php:                      || ($new_comps && (end($new_comps) == '..'))) {
./elFinderVolumeFTP.class.php:                      if ($name && $name !== '.' && $name !== '..' && substr(strtolower($info[0]), 0, 1) === 'd') {
./elFinderVolumeFTP.class.php:       * Ususaly used for images, but can be realize for video etc...
./elFinderVolumeFTP.class.php:                      if ($name !== '.' && $name !== '..' && (!$targets || isset($targets[$name]))) {
./elFinderVolumeFTP.class.php:              $excludes = array(".","..");
./elFinderVolumeLocalFileSystem.class.php:                  if (($comp != '..')
./elFinderVolumeLocalFileSystem.class.php:                  || ($new_comps && (end($new_comps) == '..'))) {
./elFinderVolumeLocalFileSystem.class.php:   * Usualy used for images, but can be realize for video etc...
./elFinderVolumeDropbox.class.php:   * Ususaly used for images, but can be realize for video etc...
./elFinderVolumeMySQL.class.php:     * Usualy used for images, but can be realize for video etc...

There are not that many results. Even less if we suppose elFinderVolumeFTP, elFinderVolumeDropbox and elFinderVolumeMySQL probably have nothing to do with our issue. Among the results, a code comment might catch the eye:

./elFinderVolumeDriver.class.php: // normalize `/../`

Following that path leads to the getFullPath function from the elFinderVolumeDriver class. This abstract class implements common components that are used by what are called drivers that abstract the file operations for the different source types. We said we would only consider the local file system case, so let's not bother with that.

/**
    * Resolve relative / (Unix-like)absolute path
    *
    * @param string $path  target path
    * @param string $base  base path
    * @return string
    */
protected function getFullPath($path, $base) {
    [...]
    // normalize `/../`
    $normreg = '#('.$sepquoted.')[^'.$sepquoted.']+'.$sepquoted.'\.\.'.$sepquoted.'#'; // '#(/)[^\/]+/\.\./#'
    while(preg_match($normreg, $path)) {
        $path = preg_replace($normreg, '$1', $path, 1);
    }

    [...]

    return $path;
}

We skipped irrelevant parts of the function to get to the heart of the question, and to the pattern we were looking for. And what we see is actually a regex that is being used to "normalize `/../`". We could ask: hey, what could go wrong?, but we already know the answer.

Thanks to the code comments, we know that the pattern being looking for is #(/)[^\/]+/\.\./# and all the matches are replaced with a /. This process is repeated until no match is found.

But this pattern is wrong. The //../ sequence will never be matched, allowing the path traversal we highlighted.

Let's fire up the "go to reference" feature of our IDE to validate this function is part of the call chain for the file action:

• Maj+F12: elFinderVolumeLocalFileSystem::_inpath
• Maj+F12: elFinderVolumeLocalFileSystem::_stat
• Maj+F12: elFinderVolumeDriver::stat
• Maj+F12: WOWOWOW

elFinderVolumeDriver calls the stat method more than 50 times in tenth of functions, including one called file (the name of the action we called on the API). Most of those functions are controllers for actions which mean our issue might have a broader impact than just reading arbitrary files.

Back to the future

At the time of our engagements, this discovery was enough to get RCE. Our target was running a Laravel instance for which we leaked the cookie signing secret. You know the sequel.

The next question that arose was: is this issue still present in the latest release? That question was legitimate as about 50 minor versions were released since the one we tested against.

$ git diff --ignore-cr-at-eol --ignore-space-at-eol -w --ignore-blank-lines 2.1.12 2.1.60 -- php/elFinderVolumeDriver.class.php
[...]
-       protected function getFullPath($path, $base) {
+    protected function getFullPath($path, $base)
+    {
        $separator = $this->separator;
        $systemroot = $this->systemRoot;
+        $base = (string)$base;

-               if ($base[0] === $separator && strpos($base, 0, strlen($systemroot)) !== $systemroot) {
+        if ($base[0] === $separator && substr($base, 0, strlen($systemroot)) !== $systemroot) {
            $base = $systemroot . substr($base, 1);
        }
+        if ($base !== $systemroot) {
+            $base = rtrim($base, $separator);
+        }

        // 'Here'
        if ($path === '' || $path === '.' . $separator) return $base;
@@ -5259,6 +6807,9 @@ abstract class elFinderVolumeDriver {
        while (preg_match($normreg, $path)) {
            $path = preg_replace($normreg, '$1', $path, 1);
        }
+        if ($path !== $systemroot) {
+            $path = rtrim($path, $separator);
+        }

The diff between the 2.1.12 and the latest version speaks for itself. No significant change was made to the vulnerable getFullPath function. Right before cracking open the champagne, we threw the exploit payload to a test deployment of the latest version.

So our vulnerable function did not change. Yet, our exploit payload does not work anymore. Sure, another change in the software must prevent the directory traversal.

Among the vulnerabilities reported by SonarSource [2], two actually are path traversal related. The _joinPath method of the elFinderVolumeLocalFileSystem was the root cause for this issue. It concatenated two paths without any checks and was patched the following way:

While the function was previously just returning a concatenation of two paths, without further validation or sanitation, the new version contains a few security measures. In particular, a call to realpath might be responsible for our problems if it happens that the _joinPath function is part of the call chain for the file action.

• Maj+F12: Direct usages of this function are found in few controllers not related to the file action. Also elFinderVolumeLocaleFileSystem::_abspath.
• Maj+F12: elFinderVolumeDriver::abspathCE
• Maj+F12: elFinderVolumeDriver::decode
• Maj+F12: pretty much everywhere

The decode method from elFinderVolumeDriver is actually the one that is used to decode the target kind of arguments (AKA from a l1_AAAA parameter, it extracts the actual file pointer). It is therefore called in most action controllers, including the one for file calls. There is no way you can go around the decode call.

However, if we go back down the call chain to _joinPath and stop on the elFinderVolumeLocaleFileSystem::_abspath, it appears it will not always call the doomed _joinPath function.

protected function _abspath($path)
{
    if ($path === DIRECTORY_SEPARATOR) {
        return $this->root;
    } else {
        if (strpos($path, $this->systemRoot) === 0) {
            return $path;
        } else if (DIRECTORY_SEPARATOR !== '/' && preg_match('/^[a-zA-Z]:' . preg_quote(DIRECTORY_SEPARATOR, '/') . '/', $path)) {
            return $path;
        } else {
            return $this->_joinPath($this->root, $path);
        }
    }
}

In fact, _joinPath is only called when the path being queried is a relative path. Using an absolute path should then be enough to exploit the vulnerability on the latest version.

Of course, just throwing the absolute path to /etc/passwd at the application won't do the trick.

The answer to the question "what happens if the path is absolute?" is in fact in the call chain to the getFullPath function. If you look back at it, you will see that the chain goes through a function named _inpath. This one is said to prevent directory traversal.

protected function _stat($path)
{
    $stat = array();

    if (!file_exists($path) && !is_link($path)) {
        return $stat;
    }

    //Verifies the given path is the root or is inside the root. Prevents directory traveral.
    if (!$this->_inpath($path, $this->root)) {
        return $stat;
    }

As it is explained in the comments, the _inpath is used to validate that the requested path is inside the configured web root directory. Even if we know this part of the software fails at handling dot dot slashes there is no chance that /etc/passwd is considered inside the root. For that to happen, the provided path needs to start with the configured root path as a prefix.

Well, that's nothing like impossible. If we build a path that actually starts with that prefix, and exploit the double slash trick to inject unsanitized ../ sequences after it, we should be able to pass all the checks and sanitation.