@alikon "traverse" - is this like #24515?

Sorry for jumping in, but #24515 is related to a tree traversal in code which does not appear to be related to this in any way.
In the context of this bugreport, "traversal" refers to a Directory traversal attack

no that pr was for fields

The issue is not about adding usage of realpath()

The issue is about adding realpath() only to 1 of the 2 directories being compared (that 1 is contained inside the other)

More specifically during comparison,
realpath() is only applied to the directory being checked as safe
it is not applied to COM_MEDIA_BASE
assuming that COM_MEDIA_BASE is not a symbolic link ?

e.g.

if (strpos(realpath($fullPath), JPath::clean(COM_MEDIA_BASE)) !== 0)

JPath::clean does return a path where realpath() has been applied
(aka symbolic links and references to /./, /../ and extra / are not resolved)

I have not tested
but it seems that using
realpath(JPath::clean(COM_MEDIA_BASE))

should fix this ???

could you create a PR @ggppdk for this?

Please note that it seems that this isn't the only location where 3.9.5 introduced issues. I run into issues with the file upload as well. I'll have to search for realpath in all the 3.9.5 patch diffs

@nonickch

yes it was an example , one place (there are others as seen by the commit that you linked to)

@HLeithner

aahh, i am sorry, i can not contribute this period, i am only partly watching the repository

@ggppdk
if (strpos(realpath($fullPath), JPath::clean(COM_MEDIA_BASE)) !== 0)

Unfortunately this is not gonna solve the issue. If the $fullPath is a symlink then the COM_MEDIA_BASE should be also the same symlink, otherwise the realpath gonna return a different absolute path.

e.g.:
$fullPath == /mnt/uploads/images -> /var/app/current/uploads/images
COM_MEDIA_BASE == /mnt/uploads/-> /var/app/current/uploads

@joel-lupfer

yes,
but i did not say it solves anything,
this particular code is a copy-paste example of existing code

i suggested modifying it, and adding realpath() to both directories

• and of course all other occurrences of similar code should be modified at the same time

see here a complete list:
1547f8e

Extending on nonickch's argument

From the phpmanual:
realpath() expands all symbolic links and resolves references to /./, /../ and extra / characters in the input path and returns the canonicalized absolute pathname.

With that said there there should be NO usage of realpath AT ALL as it "expands all symbolic links". Symlinks could end up outside of the document route and those locations would cause access problems when accessed directly. It's the the prerogative of the site-owner to do things use linking to organise / share files. Apache allows for FollowSymLinks directive to disallow "traversal", possibly nginx has something similar (dunno).

Its far more likely the intended usage was "resolves references to /./, /../ and extra / characters" which makes sense.

If the latter is true, invocations of realpath should be replaced by a function that just does "resolves references to /./, /../ and extra / characters". Short search did not reveal quick built in solution

some regexp magic instead ... The next challenge would be cover/extend the test paths

$base      = "/var/www/images/";
$testpaths = [
	'folder0//////../folder1//..'
	, 'folder0///..///../folder1//..'
	, 'folder0/../folder1/..'
	, 'folder0/../folder1/../'
	, 'folder0/../folder1/..//'
	, 'folder0/../folder1/..///'
	, 'folder0/../folder1/..///..'
	, 'folder0/../folder1/..///../'
	, 'folder0\\\\..\\\\folder1\\\\..'
	, '//////../folder1//..'
	, '///..///../folder1//..'
	, '/../folder1/..'
	, '/../folder1/../'
	, '/../folder1/..//'
	, '/../folder1/..///'
	, '/../folder1/..///..'
	, '/../folder1/..///../'
	, '\\\\..\\\\folder1\\\\..'
	, '.'
	, './'
	, '././'
	, 'folder1/././'
	, 'folder1/folder2/../././'
	, '../../folder3/folder4/././'
	, '/../../folder3/folder4/././'
	, './../../folder3/folder4/././'
	, '///../../folder3/folder4/././'
];

$s=array();                                $r=array();
$s[0]='%(//+)|(\\\\\\\\)|((/.)+(/|$))%m';  $r[0]='/';
$s[1]='%([^/]+/)(\\.\\.)(/|$)%m';          $r[1]='' ;
$s[2]='%(/\.\.)(/|$)%m';                   $r[2]='/';
echo "base::$base\n-------------------------\n";
foreach ($testpaths as $testpath)
{
	// $rp = realpath($base . $testpath) ."\n";
	$result = preg_replace($s, $r, $base . $testpath);
	printf("%-50s %-50s\n", $testpath, $result);
}

output

-------------------------
folder0//////../folder1//..                        /var/www/images/
folder0///..///../folder1//..                      /var/www/images/
folder0/../folder1/..                              /var/www/images/
folder0/../folder1/../                             /var/www/images/
folder0/../folder1/..//                            /var/www/images/
folder0/../folder1/..///                           /var/www/images/
folder0/../folder1/..///..                         /var/www/images/
folder0/../folder1/..///../                        /var/www/images/
folder0\\..\\folder1\\..                           /var/www/images/
//////../folder1//..                               /var/www/
///..///../folder1//..                             /var/www/
/../folder1/..                                     /var/www/
/../folder1/../                                    /var/www/
/../folder1/..//                                   /var/www/
/../folder1/..///                                  /var/www/
/../folder1/..///..                                /var/www/
/../folder1/..///../                               /var/www/
\\..\\folder1\\..                                  /var/www/images//
.                                                  /var/www/images/
./                                                 /var/www/images/
././                                               /var/www/images/
folder1/././                                       /var/www/images/folder1/
folder1/folder2/../././                            /var/www/images/folder1/
../../folder3/folder4/././                         /var/www/folder3/folder4/
/../../folder3/folder4/././                        /var/www/folder3/folder4/
./../../folder3/folder4/././                       /var/www/folder3/folder4/
///../../folder3/folder4/././                      /var/www/folder3/folder4/

Please don't add a complicated regex where a native PHP function works more than adequately. Please understand this was done to address a security issue (if someone else on JSST wants to expand on it at this point by all means go for it, but understand that flat out reverting the changes made in 3.9.5 has bad side effects).

Please enlighten us with this native PHP function as realpath() is "not it" because of its side effects resolving the softlink. If there is another simpeler way of doing this I'm all for it. Only discarding regexp "just because" makes no sense.

administrator/components/com_media/models/list.php line 114 change to if (strpos(realpath($basePath), JPath::clean(realpath(COM_MEDIA_BASE))) !== 0) and have a nice day. A better solution would be to define COM_MEDIA_BASE as the cleaned and resolved path (in administrator/components/com_media/media.php change line 41 to define('COM_MEDIA_BASE', JPath::clean(realpath(JPATH_ROOT . '/' . $params->get($path, 'images'))));), this probably fixes every broken comparison introduced in the 3.9.5 patch without having to change every read of COM_MEDIA_BASE to realpath(COM_MEDIA_BASE) or the fact that a lot of places are running a JPath::clean(COM_MEDIA_BASE) operation.

diff --git a/administrator/components/com_media/media.php b/administrator/components/com_media/media.php
index eddce1ab6e..7cbeb150e6 100644
--- a/administrator/components/com_media/media.php
+++ b/administrator/components/com_media/media.php
@@ -38,7 +38,7 @@ if (substr(strtolower($view), 0, 6) == 'images' || $popup_upload == 1)
        $path = 'image_path';
 }
 
-define('COM_MEDIA_BASE', JPATH_ROOT . '/' . $params->get($path, 'images'));
+define('COM_MEDIA_BASE', JPath::clean(realpath(JPATH_ROOT . '/' . $params->get($path, 'images'))));
 define('COM_MEDIA_BASEURL', JUri::root() . $params->get($path, 'images'));
 
 $controller = JControllerLegacy::getInstance('Media', array('base_path' => JPATH_COMPONENT_ADMINISTRATOR));
diff --git a/administrator/components/com_media/models/list.php b/administrator/components/com_media/models/list.php
index 2d890a7197..f0def2679b 100644
--- a/administrator/components/com_media/models/list.php
+++ b/administrator/components/com_media/models/list.php
@@ -114,7 +114,7 @@ class MediaModelList extends JModelLegacy
                $mediaBase = str_replace(DIRECTORY_SEPARATOR, '/', COM_MEDIA_BASE . '/');
 
                // Reset base path
-               if (strpos(realpath($basePath), JPath::clean(COM_MEDIA_BASE)) !== 0)
+               if (strpos(realpath($basePath), JPath::clean(realpath(COM_MEDIA_BASE))) !== 0)
                {
                        $basePath = COM_MEDIA_BASE;
                }

Should get someone going.

Your change would only work if the images directory is symlinked but not if we have symlinks under /images.

Then we have the same problem again.

+1 HLeithner,

As realpath is used not only in the mediamanager, unaverted/unrecognised side-effects of realpath() are more widespread than apparent at first glance. Admittedly these are even more edgecases but that does not make it right to not adress them.

Imho realpath() should be replaced by "our own" implementation that excludes the symlink resolution behaviour, my regexp aproach could be a start.
Attention points for bc

• Adding testing if path actually exists (not sure what realpath does thee),
• Look into support for Windows as realpath will return a windows formatted path in windows systems
• Look into the behaviour of junctions

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24539.}

+1 HLeithner,

As realpath is used not only in the mediamanager, unaverted/unrecognised side-effects of realpath() are more widespread than apparent at first glance. Admittedly these are even more edgecases but that does not make it right to not adress them.

Imho realpath() should be replaced by "our own" implementation that excludes the symlink resolution behaviour, my regexp aproach could be a start.
Attention points for bc

• Adding testing if path actually exists (not sure what realpath does thee),
• Look into support for Windows as realpath will return a windows formatted path in windows systems
• Look into the behaviour of junctions

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24539.}

Any ETA on a fix? We had to revert all our Joomla websites to 3.9.4. I think pretty much anyone who actually deploys their websites will run into this issue. And if not I'm very curious to your solution of keeping the image folder intact while installing a new Joomla version.

So you're going to try and define a fix without specifying what the supported behaviors are, basically this issue is now implicitly saying that symlinking core directories outside the root Joomla path is a supported operation. Might as well just revert the patch related to https://developer.joomla.org/security-centre/777-20190401-core-directory-traversal-in-com-media.html and tell people it is fully expected that you can perform unwanted directory traversal within com_media because any "smart" solution is going to either re-introduce the traversal vulnerability or it's going to create a problem for nested symlinked paths. Either way I'm unsubscribing from this issue because I can already smell another round of Joomla over-engineering being cooked up and I want nothing to do with it.

I think pretty much anyone who actually deploys their websites will run into this issue. And if not I'm very curious to your solution of keeping the image folder intact while installing a new Joomla version.

I have never symlinked a folder on any site I have been involved with. I do not understand what your issue is regarding updates

We use CI and don't update our Joomla's using the interface. Every deploy is a fresh installation and takes care of database migrations etc. None of the core Joomla folders are writeable. Any folders that need to be by the application are symlinked (cache,log,images). components/modules etc are installed using composer. This has worked for the past 8 years btw. (give or take)

I'm open to alternatives, but to me this seems a breaking change in a minor version. And apparently we aren't the only ones affected by this change.

From my perspective and personal opinion only - that has never been a supported method of using joomla - and just as you have modified your entire installation to match your own CI you will need to further modify it to address this issue. I certainly not in favour of joomla reducing its security for everyone because someone using a customised installation doesn't like it.

Its pretty common to symlink folders if you have more then on stage. Also having Joomla readonly is much better then joomla can offer so the usecase is vaild from my point of view.

It still has never been a supported or documented means of using Joomla

Maybe in Future a Feature if this is common as @HLeithner say?

I doubt it is very common as it is impossible on most shared hosts which is where most joomla sites are installed.

As for a feature - the media manager in j4 is completely different

IMHO symlinking is pretty common practice to seperate code from user created data. It is used a lot at the managed hosting companies.

IMHO symlinking is pretty common practice to seperate code from user created data. It is used a lot at the managed hosting companies.

Back that up with real usage data from joomla sites not any other system please

Brian why this pointless arguing ...

Symlinking is a perfectly legal way to "build" your structure, its supported by linux and by apache so why are we even arguing on its usage. Its legal and its not for us to argue if it makes sense, as with anything it will differ for different people.

How can I back that up? By providing a list of sites or companies which are hosted this way?

The only thing I can see is that there are multiple people here arguing in favor and only one against.

@marcodings my point is to counter the statements that this is a very common practice - it is not because it can not be for the reasons I stated above.

If you want a fix then roll up your sleeves and contribute one.

Symlinking is a perfectly legal way to "build" your structure, its supported by linux and by apache so why are we even arguing on its usage.

Because in effect Joomla does not natively support being deployed as a 100% read-only filesystem (this prevents the core CMS from updating itself or its extensions, and as Joomla does not support an upgrade mechanism outside the web interface any other option (including my save your ass gist) is at your own risk). Because in effect Joomla does not support being deployed in an environment where internal application paths are symlinked to a location where its canonical path is outside JPATH_ROOT, the entire application is expected to be in one space.

The one "security" trick you might be able to get away with so long as every extension feature is audited is to have the entire Joomla application in a non-web accessible directory and symlink the required bits and pieces (administrator/templates/*/(css|html|images), administrator/index.php, images, index.php, media, and templates/*/(css|html|images) should be the main paths with publicly accessible files) into the web space, that scenario should not be broken by the changes in 3.9.5 if everything is coded correctly.

For anyone who has looked at the official Joomla Docker image, there is a reason I purposefully designed the image to make the entire application a volume and not have the image support upgrading an existing Joomla application; image updates only handle upgrading the underlying server dependencies and for brand new installations provision the Joomla package.

components/modules etc are installed using composer

This too is not a core Joomla supported workflow (or for that matter most extensions) and you're on your own if you're trying to deploy the CMS in this manner.

====

It sounds like there are a lot of people who are using workflows which are not supported by the core Joomla project for managing their websites. A lot of these workflows are pretty similar to my daily Symfony and Laravel deployments, I am not unfamiliar with your routines so please leave the insults at the door. But, these workflows do not mesh with how Joomla as an application is developed and intended to be deployed and managed. If you want to see Joomla officially supported as a Composer oriented application or support symlinked symlinks to all the places, then there is a lot of work that needs to be done in core and coming in here complaining that an unsupported workflow was busted by a security change is not how you get anything fixed.

As always @mbabker explains it more eloquently than me

@brianteeman >If you want a fix then roll up your sleeves and contribute one.
so what do my posts here suggest ?

@mbabker

Joomla does not natively support being deployed as a 100% read-only filesystem
Joomla does not support an upgrade mechanism outside the web interface
I don't see me suggesting that in any way shape or form?

I'm arguing that symlinks are legal file constructs and they allow for building a structure below JPATH_ROOT.
The canonical path ( where it resolves symlinks or mounts or ?) is not relevant for the security context of breaking out of JPATH_ROOTor gaining access to other parts of the system, potentially overwriting adding files. Removing ../ and the links would serve just fine.
It's up to the person creating the structure to make sure he/she has not intoduced side effects.

Long story short.. BC break in behaviour was introduced and we have to deal with that, not discuss if these usecases were 'ok' in the eye of the beholder.
We should not judge the workflow as such if the result is a valid file structure i think we should deal with it.

@mbabker

Joomla does not natively support being deployed as a 100% read-only filesystem
Joomla does not support an upgrade mechanism outside the web interface
I don't see me suggesting that in any way shape or form?

I'm arguing that symlinks are legal file constructs and they allow for building a structure below JPATH_ROOT.
The canonical path ( where it resolves symlinks or mounts or ?) is not relevant for the security context of breaking out of JPATH_ROOTor gaining access to other parts of the system, potentially overwriting adding files. Removing ../ and the links would serve just fine.
It's up to the person creating the structure to make sure he/she has not intoduced side effects.

Long story short.. BC break in behaviour was introduced and we have to deal with that, not discuss if these usecases were 'ok' in the eye of the beholder.
We should not judge the workflow as such if the result is a valid file structure i think we should deal with it.

because a behavior is not document doesn't mean we have to break it.

Security disclosure time then, since it seems some people want to argue semantics and don't seem to grasp basic security of a web application with an end user facing interface that allows interaction with the application's filesystem.

In Joomla versions 3.9.4 and earlier, with well crafted URLs, it was possible for a malicious visitor to use the media manager to traverse the entirety of a Joomla installation, and using the media manager's features this would enable them to upload or delete files anywhere within the application.

The fix introduced in 3.9.5 patches the media manager in such a way that it is no longer allowed to operate outside the path it is configured to use (generally the root images directory unless someone really wants to be ballsy and use a directory other than "images"). As a result, an improper filesystem check causes the user to be unable to navigate inside directories of a symlinked images directory, the changes I suggested above deal with that specific issue and should be applied regardless of your arguments of semantics.

because a behavior is not document doesn't mean we have to break it.

When a behavior is not documented as supported, that means it is not expected that it is catered for when patching bugs (especially those of a security nature). Therefore, it is perfectly valid to state that a non-core supported workflow being broken as a result of a patch, while being a crappy scenario to land in, is not core's responsibility to fix unless it is going to document that workflow as a supported feature. Otherwise, it can be argued that every bug fix is a B/C break because someone's code relied on either an undocumented behavior or a documented broken behavior.

It is impossible to make sure you don't break something if that something is something it was not designed to do.

Again, for clarity, this specific patch fixes the inability for an end user to navigate into a directory inside a symlinked images directory. It does not fix any issue where a directory inside the images directory is symlinked elsewhere (which includes a scenario where the images directory is symlinked to one place and one of its directories are symlinked someplace else).

Where it was commented about having symlinks for other "user content" directories such as the cache, logs, or tmp paths, those should be configured in the configuration.php file with the appropriate properties instead of being symlinked (though word of caution about moving the cache directory outside the web space since extensions regularly published web accessible assets to that path).

diff --git a/administrator/components/com_media/models/list.php b/administrator/components/com_media/models/list.php
index 2d890a7197..f0def2679b 100644
--- a/administrator/components/com_media/models/list.php
+++ b/administrator/components/com_media/models/list.php
@@ -114,7 +114,7 @@ class MediaModelList extends JModelLegacy
                $mediaBase = str_replace(DIRECTORY_SEPARATOR, '/', COM_MEDIA_BASE . '/');
 
                // Reset base path
-               if (strpos(realpath($basePath), JPath::clean(COM_MEDIA_BASE)) !== 0)
+               if (strpos(realpath($basePath), JPath::clean(realpath(COM_MEDIA_BASE))) !== 0)
                {
                        $basePath = COM_MEDIA_BASE;
                }

Again, for clarity, this specific patch fixes the inability for an end user to navigate into a directory inside a symlinked images directory.

I also ran into problems with this security fix. I also have my images directory symlinked since I share it with multiple websites.

This fix works great, and really improves things. Can we include it in 3.9.6?
I understand it doesn't fix all cases, but why not have it if it at least improves some of the cases?

@smehrbrodt there is no Pull Request > no 2 successfully tests > no merge.

@mbabker as you are the patch author, would you mind submitting a pull request? Or should I do that?

Feel free to send the PR. I was just offering a quick fix for the most basic problem. Doesn't fix every potential use case but it's still better than what's there now.

Closed as having Pull Request.

The PR doesn't solve this problem completely.

This issue seems to be the same issue as I reported as
TinyMCE drag & drop - "Unable to upload file" error #24877.
Thanks.

Set to "closed" on behalf of @Quy by The JTracker Application at issues.joomla.org/joomla-cms/24539

Please test PR #24924.

Please test PR #24924.

It still has never been a supported or documented means of using Joomla

This is kind of a nonsensical statement. Symlinks are a really normal and common thing. You don't need to explicitly state that you support them, you need to explicitly state that you do not support them if that's the case.

@okonomiyaki3000 if you are interested to help please test #24924

@HLeithner will do

The patch looks good. I understand it that only solves the issue that the root media folder is a symlink and not the issue that some nested folder may also be one. That issue is not something I'm personally concerned about but, if it's important enough to require a solution, how about adding a whitelist of paths to the com_media options. No need for some complex logic to determine the safety of paths, just let the user define what is safe (he should know, right?). Maybe could even allow glob patterns... I suppose that would be enough for anyone's needs.

The patch looks good. I understand it that only solves the issue that the root media folder is a symlink and not the issue that some nested folder may also be one. That issue is not something I'm personally concerned about but, if it's important enough to require a solution, how about adding a whitelist of paths to the com_media options. No need for some complex logic to determine the safety of paths, just let the user define what is safe (he should know, right?). Maybe could even allow glob patterns... I suppose that would be enough for anyone's needs.

Maintaining a white list of media paths seems to be a good solution for now.
(My Joomla installation has a symlink subfolder under 'images' folder of Joomla installation. So the current patch cannot work for my case.)

Thanks.

I think we will take this solution for j3 if we don't find a better solution, j4 has a new mediamanger with self defined basefolders incl. External storage providers.

sorry, misreaded.

I think we will take this solution for j3 if we don't find a better solution, j4 has a new mediamanger with self defined basefolders incl. External storage providers.

Hi, I have a symlink'ed subfolder under images folder.
"/J_installation/images" is a physical path, and 'images' folder has a few physical subsolders as well as a symlink subfolder like "/J_installation/images/media" --(symlink)--> "/media".
The recently released Joomla 3.9.8 didn't resolve my problem. (failed to navigate into 'media' subfolder)

Will this symlinked subfolder be supported in future?
Or, should I reorganize my websites to fix the media manager issue?

Thanks.

I really think we should add a whitelist. I don't think it would be difficult to implement at all.