Template XML retrival fails if USER_FILES is a symbolic link

[uoe-pjackson]

Since upgrading from 3.10.5 to 3.12.29 the browser Editor isn't working. After digging into the details it appears that the request being made for get_template_xml.php is returning Not found!

e.g https://xerte.example.com/website_code/php/templates/get_template_xml.php?file=USER-FILES/7-pjackso1-site/preview.xml&time=1721060715679

I've added in a var_dump($full_unsafe_file_path, $realpath); to get_template_xml.php as follows

$unsafe_file_path = $_GET['file'];

$full_unsafe_file_path = $xerte_toolkits_site->root_file_path . $unsafe_file_path;

// This gets the canonical file name, so in case of 542-tom-Notingham/../../../../etc/passwd -> /etc/passwd
$realpath = realpath($full_unsafe_file_path);
// Check that is start with root_path/USER-FILES
if ($realpath !== false && $realpath === $full_unsafe_file_path) {
    echo file_get_contents($realpath);
}
else{
    var_dump($full_unsafe_file_path, $realpath);
    echo "Not found!";
}

The output shows the mismatch between the expected and realpath

string(62) "/apps/lamp/xerte/htdocs/USER-FILES/7-pjackso1-site/preview.xml" string(52) "/remote/xerte/USER-FILES/7-pjackso1-site/preview.xml" Not found!

It appears that the cause of the issue is USER-FILES directory being a symbolic link to /remote/xerte/USER-FILES. We've done this to have multiple app servers connecting to a shared filesystem server.

It appears that this is the same issue as described in https://www.xerte.org.uk/index.php/en/forum/installation/3201-new-installation-unable-to-edit-preview-project?start=6#8812

[uoe-pjackson]

There doesn't look like a simple fix for having the USER-FILES as a symbolic link, whilst using the realpath. To allow this to work I can think of a couple of different options.

Option 1

Remove the realpath() call in website_code/php/templates/get_template_xml.php and getfile.php and instead do checking of the file variable path for path traversal by checking for .. taking into account double encoding attacks and similar.

Option 2

Create a new config setting called for example: users_file_area_path that contains the filesystem path to where the files are located e.g "/remote/xerte/USER-FILES", if that is not set in the database then it defaults to the current value of: $xerte_toolkits_site->root_file_path . $xerte_toolkits_site->users_file_area_short

Then when the file is being checking using realpath it's checked against the new $xerte_toolkits_site->users_file_area_path I think it may be necessary to ignore an extra "USER-FILES" that with be both in the file variable and in users_file_area_path.

Option 2 is more involved including a new column into sitedetails tables but is probably a more robust solution than option 1. Before I work on a patch for either of these it would be good to get an indication which of these would be preferred by Xerte Project development community?

Thanks

[torinfo]

@uoe-pjackson Thenk you for reporting this and providing a solution and yoour willingness to provide a patch.

For the next release we are working on tightening up Xerte even more, so there will be much more locations where we are using realpath. So this will become an issue. So your reporting is at the ciorrect time to prevent much bigger issues in the next release.

My proposal is to rename the field users_file_area_short to users_file_area in the database and to allow that field to have an absolute path (or the current releative path)

Than, I would create a function get_user_area_path() that will build the actual path.

Also I propose to create a function check_user_path() to check whether paths have a path traversal issue. This way, if there is anything wrong with the check, we can fix it in one place.

So, for now, I would suggest you choose Option 1 to fix it for you installation.

We will test this (create an installation with a symbolic link) before the next release.

Also, I will check and remove all direct references to USERS_FILES in teh code, so you might actually use a path like /remote/xerte/projects

Would that work for you?

[uoe-pjackson]

@torinfo That sounds like a good plan. We've already done an option 1 fix as you suggest and await later versions that have a better solution.

Many Thanks Peter