Magento-lts: Ability to create new directories on the server via URL

[Krossfire]

Preconditions (*)

1. OpenMage 19.4.8 (also able to recreate on any version of Magento, since 1.7.x)
2. Linux-based server (tested on Ubuntu, Debian, and CentOS)
3. Apache 2.4.41
4. Rewrites enabled (.htaccess)
5. Files chmod-ed to 644
6. Directories chmod-ed to 755
7. PHP 7.3.19 and PHP 7.2.34

Steps to reproduce (*)

1. Install a fresh instance of OpenMage 19.4.8 (or use any existing instance)
2. Enable rewrites (not sure if required)
3. Go to https://your_instance_domain/media/js/dir1/dir2/dir3

Expected result (*)

1. Get the regular 404 error message/page

Actual result (*)

1. Got the browser's 404 error message
2. On the server, there are now new directories: /magento_root/media/js/dir1 and /magento_root/media/js/dir1/dir2

[Flyingmana]

Thank you a lot. I did not validate this yet, but that sounds like a serious issue.
You should report this also to MageOne (if not already done) https://mageone.atlassian.net/wiki/spaces/MOBB/overview
Because this can easy be used for a Deny-Of-Service attack.

[Flyingmana]

in a full default setup, all calls to /media/ have a fallback to /get.php

https://github.com/OpenMage/magento-lts/blob/1.9.4.x/media/.htaccess#L24

[riconeitzel]

I just tried it on a regular customer instance and wasn't able to reproduce the behavior either. I'll check again with a fresh copy in a controlled environment.

[ADDISON74]

I CONFIRM THIS BUG IN OPENMAGE 20.04!

Please note I don't use any SSL termination on the test server just http://. If I enter into the browser address an URL formed like this http://www.mydomain.tld/media/ by adding the following paths to the URL:

captcha/dir1/dir2
catalog/dir1/dir2
css/dir1/dir2
dhl/dir1/dir2
js/dir1/dir2

I was able to create dir1 only (not dir2) in all folders above. The following folders make an exception from the rule::

customer
downloadable
tmp
xmlconnect

If the path is longer you can create a tree of folders minus the last one (e.g. /media/catalog/dir1/dir2/dir3/dir4 => /media/catalog/dir1/dir2/dir3).

With a basic script running long enough you can get a DOS on the server. File get.php should be revised by an expert ASAP.

[riconeitzel]

Even in a controlled environment with a vanilla Magento 1.9.4.5 this behavior is not reproducible here. Might be different with older versions and/or open mage version.

[qsolutions-pl]

I was not able to reproduce this in any of M1 stores from our Customers.

[Krossfire]

I see that get.php:148 calls the:
Mage_Core_Model_File_Storage_File::lockCreateFile($filePath)
which then calls:
Mage_Core_Model_Resource_File_Storage_File::lockCreateFile($filePath)
and this is where the directory is created (lines 245 - 247):
if (!is_dir($path)) { @mkdir($path, 0777, true); }

[ADDISON74]

On the same server (Debian 10 with Virtualmin) I installed from scratch in different directories 3 versions of Magento CE (https://pubfiles.nexcess.net/magento/ce-packages/) and OpenMage 20.0.4.

• 1.9.2.4 - OK
• 1.9.3.10 - OK
• 1.9.4.5 - OK

After installing OpenMage 20.0.4 I used in my browser as address http://www.mydomain.tld/media/dhl/dir1/dir2. Checking the file system I found dir1 in /media/dhl.

Please note that always you need to use a second folder in path to create the first one. I created a basic bash script that runs in a loop a curl command. I was able to create a tree of folders. This is not good at all for OpenMage.

In conclusion, there is no doubt this is OpenMage serious bug only!

[ADDISON74]

Well, I found a fix for this issue.

I renamed in root the file get.php then I copied the same file from Magento CE 1.9.4.5. By using this vanilla code I was not able to reproduce this issue anymore.

We should do a DIFF between Magento and OpenMage code related to this file to see what happened between versions.

Thank you @Krossfire for finding this bug. It is a must to check more closely the changes made in the new versions of OpenMage. Each implementation should be done after long tests.

[Flyingmana]

ok, if its OpenMage only, than its likely this PR which introduced it https://github.com/OpenMage/magento-lts/pull/601

[Flyingmana]

So how can we prevent this:
We could just clean up all the empty folders, that sounds easy. But not so easy to do efficiently, as it needs to check first if the folder is empty.

so its probably better to change the locking to something which does not need directories. We could use a hash function on the path, or strip all directory separators. And maybe we should not store the lock inside the media directory, although this may make things harder for multi server environments which use a shared media.

Maybe we can get input from @colinmollenhour what in his initial usecase would work best.

[colinmollenhour]

Yes, this issue was indeed introduced by my patch in #601 and I agree this is an unintended and unacceptable side-effect. My bad!

For a quick protection you could add a cron task like so to cleanup periodically:

0 * * * * find /path/to/media -empty -type d -delete

I don't know how many directories one would need to create to inflict some damage, it would vary by file system, free space, etc.. But probably tens of millions at least so an hourly cleanup should be sufficient. Of course a WAF and rate limiter would help slow this attack and is recommended to have regardless.

The purpose of the locking introduced in #601 is to fix potential stampeding of remote fetching which could consume a lot of resources (arguably magnified a lot more than creating empty directories), so it creates the file and locks it before fetching the file contents. This prevent multiple processes from fetching the same file at the same time. Unfortunately if the file turns out to not exist in remote storage the parent directories have already been created which is not something that I considered.

Some possible solutions:

1. Keep track which directories were created by lockCreateFile and remove them in removeLockedFile (see #1338)
2. Change the locking to use a dummy file that does not use parent directories rather than the actual file. This may be less optimal than the original solution though.. Would need to think it through and test.

I pushed #1338 as a quick fix for review. Not tested.

[ADDISON74]

@colinmollenhour: I appreciate your quick reaction to this issue.

Will this temporary fix influence the execution time? I did not test it yet but we should evaluate if the modifications #601 + #1338 bring real benefits comparing with vanilla Magento code.

[colinmollenhour]

The quick fix will not add any significant time to processing, it definitely doesn't traverse directories which should be avoided. The cleanup part only happens if the file did not exist on remote storage so would only happen either in edge cases or malicious cases.

I never tested restoring the original get.php file so it may very well work to do that. The #601 update does offer some significant benefits though in cases where there are large numbers of requests that require fetching the file from remote storage.

Using a different file for the lock file as @Flyingmana suggested could certainly be a better solution but I wanted to provide a quick fix first.

[ADDISON74]

I never tested restoring the original get.php file so it may very well work to do that. The #601 update does offer some significant benefits though in cases where there are large numbers of requests that require fetching the file from remote storage.

Restoring to the original get.php fixed this issue. The other files you modified with #601 are not changing anything. You have more experience with M1 than most of us and if there are benefits please find the best solution for this improvement.

Using a different file for the lock file as @Flyingmana suggested could certainly be a better solution but I wanted to provide a quick fix first.

This sounds like indexing process when files are temporary stored in /var/locks/*.lock.

[ADDISON74]

I did a new test related to this issue by creating a folder named magento into the media directory then I accessed it in browser like this: https://www.mydomain.tld/media/magento/dir1/dir2/. There were no folder dir1 created into the /media/magento.

I renamed magento folder to js and then to captcha which are Magento known folders and I was able to create dir1 into them. My personal opinion is this issue is related only to Magento folders. Does anyone know why this is affecting only a few folders?