Affecting SuiteCRM < v7.14.2, v7.12.14

When reviewing PHP projects, one of the quick wins that I like to do is to search for logic related to file inclusion. In PHP, file inclusion can be performed by using expressions like require() or include(). These expressions essentially import the content of another PHP file at the location where the inclusion is called.

While auditing the SuiteCRM codebase I discovered files called SubpanelCreates.php and SubpanelEdits.php with some interesting logic. In both of these files, the application dynamically builds an import path using user-supplied input. As a researcher, this is intriguing, because if we can control the path, we may be able to execute arbitrary PHP code.

Interestingly, another researcher (crackcat) beat me to this finding a few months before me. In their vulnerability report ( - they highlight that line 44 calls return_module_language() and uses the user-controlled parameter target_module without sanitization which leads to a limited path traversal vulnerability. Based on crackcat’s research - if you follow this method logic this user-controlled input ends up concatenated into a file inclusion statement. Due to the concatenation required to build the module path - Crackcat’s PoC requires that the malicious file to be included is named language/en_us.lang.php. This file would need to exist somewhere else on the file system and host some type of arbitrary PHP payload. To do this, you would need to have host access to write a file somewhere on the filesystem or leverage an existing service like FTP or Samba on the host to perform an abitrary file write.

After reporting, SuiteCRM maintainers pushed a fix that introduces isAllowedModuleName to check module names for the existence of . or / characters, common path traversal sequences. The return_module_language method uses this new function for validation. With this addition, both parties confirmed that the vulnerablity was no longer reproducible.

However, after further research I discovered that this function is only called once and that is only during the return_module_language() call.

// Jenny - Bug 8119: Need to check if $module is not empty
    if (empty($module) || !isAllowedModuleName($module)) {
        $GLOBALS['log']->warn('Variable module is not in return_module_language, see more info: debug_backtrace()');

        return array();

As a vulnerability researcher, it is always interesting to see single use methods created for the purpose of generic santization like this. Instead of repeatable patterns throughout the code base and without an agreed upon development standard for contributions - developers may not reuse already existing patterns and instead opt to introduce their own validations for the specific feature they are working on.

This lack of organization is especially a problem for larger projects as maintainers come and go and it becomes harder to enforce a unified project standard.

A bunch of single use validation logic and overlapping sanitization techniques is a signal for a potentially systemic issue in the codebase.

Luckily for us, we don’t have to look far to find an example of this as the next line (line 45) contains another reference to our user input without sanitization from the previous fix.

$target_module = $_REQUEST['target_module']; // target class

If we review lines 51-64 we can see a logic block that is essentially checking to see if the target module specified exists and if there is a file called EditView.php at that path. If that file exists, the logic also checks that another file called QuickCreate.php is also present. If that file is present, a require_once is called to include that PHP file.

if (file_exists('modules/'. $_REQUEST['target_module'] . '/EditView.php')) {
    $tpl = $_REQUEST['tpl'];
    if (is_file('modules/' . $target_module . '/' . $target_module . 'QuickCreate.php')) { // if there is a quickcreate override
        require_once('modules/' . $target_module . '/' . $target_module . 'QuickCreate.php');
        $editviewClass     = $target_module . 'QuickCreate'; // eg. OpportunitiesQuickCreate
        $editview          = new $editviewClass($target_module, 'modules/' . $target_module . '/tpls/' . $tpl);
        $editview->viaAJAX = true;
    } else { // else use base class
        $editview = new EditViewQuickCreate($target_module, 'modules/' . $target_module . '/tpls/' . $tpl);
    echo $editview->display();

Since isAllowedModuleName() is only called to validate path traversal sequences in the call to return_module_language() - no validation is performed on this logic.

Since we still control the value of $target_module, and it is not subject to the validation routine introduced with the previous fix, we can abuse path traversal here to perform a similar attack.

Similar to crackcat’s PoC - the exploitation for this requires that 2 files exist somewhere on the filesystem and are accessible to the web user.

EditView.php doesn’t have to contain any content and must just simply exist to pass the first validation check.

if (file_exists('modules/'. $_REQUEST['target_module'] . '/EditView.php')) {

Next, QuickCreate.php must also exist in the same directory to pass the next check.

if (is_file('modules/' . $target_module . '/' . $target_module . 'QuickCreate.php')) { // if there is a quickcreate override

If the previous check proceeds, we now reach our target - a file inclusion.

require_once('modules/' . $target_module . '/' . $target_module . 'QuickCreate.php');

QuickCreate.php should contain our malicious payload. We can send a POST requested formatted like so to trigger our malicious payload at /home/user/Desktop/QuickCreate.php

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