[bug] Scanning dirs up the computer root and hanging browser In /lib/plugins/acmenu

[Former user]

There is an endless loop where you move scanning directories all the way up to the computer root. That almost hangs browser and scares users looking like malware activity.

In ./lib/plugins/acmenu/syntax.php see string 213-214

            while ($dir !== $conf["savedir"]) {
                $files = scandir($dir);

You requested $path = $INFO["filepath"];

so it is something like "C:/OpenServer/domains/dokuwiki/data/pages/wiki/menu.txt"

so $dir would be "C:/OpenServer/domains/dokuwiki/data/pages/wiki/"

and you compare it with $conf["savedir"] which is "./data"

It will never match.

[D. C. Stoyanov]

I agree with you that this while loop sucks, so I will improve it.

[D. C. Stoyanov]

Issue reported in https://www.dokuwiki.org/plugin:acmenu

:!: This plugin is written incorrectly in some ways, and it might be dangerous to use. I suggest to refactor its code. When I tried to use it, syntax.php _get_base() function started an endless loop, roaming all the directories on my server up to the root, scanning them again and again, and almost hung up my browser with warning messages. It thought it was malware in this plugin. Mr. Stoyanov, I would change the way you work with data. This code from that function can have side effects:

$path = $INFO["filepath"];  // <srv-path>/<data>/pages/<dir>/<file>.txt
$dir = str_replace(basename($path), "", $path);

Imagine that DokuWiki is installed in

/usr/share/nginx/html.txt/wiki/,

and you currently view a wiki page named html. So $path = $INFO["filepath"] gives you

/usr/share/nginx/html.txt/wiki/data/pages/html.txt

Then, when you str_replace, obviously you replace two parts of the path instead of one. You get dir that does not exist.

When you do operations with data, please make sure you process exactly those data which you actually meant to address, and wouldn't touch anything else.

Do not use such strange methods to find file's dir. There are many other ways in PHP to analyse dir structure, like explode-implode, dirname(), etc. In this case, we can do that:

$dir = pathinfo($path)['dirname'];

You could say that side effects would be rare, as people rarely use .txt in folder names, but we shouldn't rely on thoughts that your program "hopefully will not crash". See the next lines of your code:

// prevent searching in the attic folder when open an old revision
if (strpos($dir, '/attic/') !== false) {
    $dir = str_replace("/attic/", "/pages/", $dir);
}

Again, if the wiki has attic in its path, there will be the same problem. We shouldn't rely on a hope that nobody would put DokuWiki in '/usr/share/nginx/attic/ folder or the like.

And here is the problem I actually met with:

if (file_exists($dir) == true) {
    // this the tree path searched:
    // <srv-path>/<data>/pages/<dir>/
    // <srv-path>/<data>/pages/
    while ($dir !== $conf["savedir"]) {

You compare $dir which in my case is C:/OpenServer/domains/dokuwiki/data/pages/wiki/ with $conf["savedir"] which is ./data. No wonder that we go into an endless loop, scanning all the directories up to the root:

    while ($dir !== $conf["savedir"]) {
        $files = scandir($dir);
        if (in_array($conf["sidebar"] . ".txt", $files) == true) {
            $re = "/(.*\/pages\/)/";
            $base_ns = preg_replace($re, "", $dir);
            $base_ns = str_replace("/", ":", $base_ns);
            break;
        }
        else {
            $re = "/" . basename($dir) . "\/$/";
            $dir = preg_replace($re, "", $dir);
        }
    }

Instead of $conf["savedir"] use realpath($conf["savedir"])

Moreover, you should never scan the entire dir if you just want to know if one file exists.

And those preg_replaces... Firstly, they shouldn't be used in a loop. Secondly, they shouldn't be used at all. --- [[user>sancaya|Constant Illumination]] //2018-11-10 16:24//

[D. C. Stoyanov]

I wish it to be clear that this plugin is not a malware and files are neither deleted nor stolen in any case. However, there is a bug due to a while statement which enters in an infinite loop if your wiki is under a "non-standard" directory and causing your browser to be unreactive. If you think to be in this situation or if you don't want to risk, do not install this plugin until this warning note remains here.

--- [[[e-mail]|Torpedo]] //2018-11-12 14:03//

[D. C. Stoyanov]

fixed #18

[D. C. Stoyanov]

Removing version: v1.0 (automated comment)