Common Security Vulnerabilities in Craft CMS Plugins

October 31, 2021
by Ben Croker

I’ve said it before and I’ll say it again: never trust user input. This is one of the golden rules of security, yet when designing plugin APIs, we often tend to favour flexibility over restriction, which can get us into trouble if we are unaware of the data or endpoints we are exposing.

Halloween

Below are the most common types of security vulnerabilities I see in Craft CMS plugins. As you’ll likely notice, all are caused by exposing too much to the user without first validating they have the necessary permissions. Therefore the best practice in terms of security is to lock things down by default and only expose what is necessary, as restrictively and explicitly as possible.

Sensitive Data Exposure #

Processing user input in the form of query string parameters may at first seem convenient and relatively harmless. But beware – here there be tygers!

Tiger

Say you wanted to create an API that exposes content in your CMS, such as entries, categories and tags. You opt for a flexible approach, whereby you allow the user to select the type of element they wish to retrieve using a query string parameter. The resulting elements are then returned as a JSON encoded string.

use Craft;
use craft\web\Controller;
use yii\web\Response;

class ApiController extends Controller
{
    public function actionGetElements(): Response
    {
        $results = [];
        $elementType = Craft::$app->request->getParam('elementType');

        if ($elementType) {
            // Fetches the element query dynamically.
            $elementClass = '\\craft\\elements\\' . $elementType;
            $elementQuery = $elementClass::find();
            $results = $elementQuery->all();
        }
      
        return $this->asJson($results);
    }
}

So when the URI contains?elementType=Entry, all entries are returned in JSON format. Similarly, the URIs ?elementType=Category and ?elementType=Tag will output all categories and tags respectively.

[
  {
    id: 10,
    title: "My Entry",
    slug: "my-entry",
    uri: "my-entry",
    sectionId: "1",
    typeId: "1",
    authorId: "1",
    postDate: {
      date: "2021-10-31 10:47:00.000000",
      timezone_type: 3,
      timezone: "Europe/Vienna"
    },
    ...

This is a permissive API that allows users to select the type of content they want to fetch. However, it exposes all element types, not just the options provided by the select field. It is trivial to modify the content of the select field in the browser’s dev tools. It is even easier to modify the URI directly.

This approach can be exploited by adding an unintended elementType to the query string. For example, the URI ?elementType=User will expose all user data, which you certainly would never want to do (fortunately Craft obfuscates user passwords).

[
  {
    username: "ben",
    photoId: 11,
    firstName: "Ben",
    lastName: "Croker",
    email: "[email protected]",
    password: null,
    admin: "1",
    locked: "0",
    suspended: "0",
    pending: "0",
    lastLoginDate: {
      date: "2021-10-31 10:21:19.000000",
      timezone_type: 3,
      timezone: "Europe/Vienna"
    },
    ...

To avoid this, we can create a list of allowed entry types and only proceed if one of them is selected.

public function actionGetElements(): Response
{
    $results = [];
    $allowedElementTypes = ['Entry', 'Category', 'Tag'];
    $elementType = Craft::$app->request->getParam('elementType');

    // Ensures that `$elementType` exists in the allowed list.
    if ($elementType && in_array($elementType, $allowedElementTypes)) {
        $elementClass = '\\craft\\elements\\' . $elementType;
        $elementQuery = $elementClass::find();
        $results = $elementQuery->all();
    }

    return $this->asJson($results);
}

Perhaps a more explicit way to do this is using a switch, which has the added benefit of allowing us to run specific logic per element type. For example, say we wanted to allow the filtering of entries by section, we could add that logic to the Entry case only.

public function actionGetElements(): Response
{
    $results = [];
    $elementType = Craft::$app->request->getParam('elementType');

    switch ($elementType) {
        case 'Entry':
            $elementQuery = \craft\elements\Entry::find();
            $section = Craft::$app->request->getParam('section');
            // If a section is provided then filter the entries by that section.
            if ($section) {
                $elementQuery->section($section);
            }
            break;

        case 'Category':
            $elementQuery = \craft\elements\Category::find();
            break;

        case 'Tag':
            $elementQuery = \craft\elements\Tag::find();
            break;
    }

    if (!empty($elementQuery)) {
        $results = $elementQuery->all();
    }

    return $this->asJson($results);
}

It’s quite likely that we’d only want to expose some sections. Well, we can limit them using an allowed list, just as we did before with allowed entry types.

    switch ($elementType) {
        case 'Entry':
            $allowedSections = ['albums', 'books'];
            $section = Craft::$app->request->getParam('section');
            // Ensures that `$section` exists in the allowed list.
            if ($section && in_array($section, $allowedSections) {
                $elementQuery = \craft\elements\Entry::find()->section($section);
            }
            break;
            
        case 'Category':

In reality, you’d most likely want to be more explicit about which attributes are returned, rather than returning entire elements, which can be achieved with just a bit more code.

    if (!empty($elementQuery)) {
        foreach ($elementQuery->all() as $element) {
            $results[] = [
                'id' => $element->id,
                'title' => $element->title,
                'slug' => $element->slug,
            ];
        }
    }

    return $this->asJson($results);
}

Controller Action Permissions #

Controller actions, by default, can only be called by logged-in users. But you can specify in your plugin that a controller should allow anonymous access by overriding the $allowAnonymous property, which can accept several values. Each value determines which users can access what and when, so understanding the effects of the value you use is crucial.

I am bad ass

By setting $allowAnonymous to true, you make all of the controller actions anonymous when the system is live. 

use craft\web\Controller;

class ApiController extends Controller
{
    /**
     * Setting this to `true` or `self::ALLOW_ANONYMOUS_LIVE` indicates that all 
     * controller actions can be accessed anonymously when the system is live.
     */
    protected $allowAnonymous = true;

While this may be the desired result, it is also possible to be more explicit about which actions should be anonymous, so that you don’t accidentally expose actions to unauthenticated users.

use craft\web\Controller;

class ApiController extends Controller
{
    /**
     * Setting this to an array of action IDs indicates which actions can be 
     * accessed anonymously when the system is live.
     */
    protected $allowAnonymous = ['save-guest-entry', 'edit-guest-entry'];

You should be very careful about which controller actions you make anonymous, as you are essentially opening them up to the entire Internet. 

But it is not just anonymous controller actions that need to be considered. Remember how we agreed that user input should never be trusted? Well, that includes authenticated users too. Many sites offer free user registration that only requires an email address. If you have a controller action that exposes or modifies sensitive data then you should use request validation methods to ensure that logged-in users have the necessary permissions to run actions.

public function actionGetSensitiveData(): Response
{
   // Throws an error if the current user is not an admin.
   $this->requireAdmin();

   ...
}

If you’ve registered user permissions in your plugin then you can verify that a user has permission to access a specific action.

public function actionGetSensitiveData(): Response
{
   // Throws an error if the current user does not have the required permission.
   $this->requirePermission('getSensitiveData');

   ...
}

Uncontained Path Access #

Some plugins provide the ability to read from or write to a directory whose path may depend on user input. Let’s say we want to provide a controller action for downloading images that exist in an image assets folder.

public function actionDownload(): Response
{
    $file = Craft::$app->request->getRequiredParam('file');
    $filePath = Craft::getAlias('@webroot/assets/images/' . $file);

    return Craft::$app->response->sendFile($filePath);
}

Given a file parameter, this controller action prompts a download of the file in the images directory. Seems harmless, right? Wrong!

Wrong way

The URI ?file=cat.jpg will correctly download the cat pic located at @webroot/assets/images/cat.jpg. But what happens if a maliciously crafted URI such as ?file=../../index.php is requested? You guessed it, the dot-segment .. indicates go up one level” in the path, meaning that we can break out of the images folder and even the assets folder into the webroot. In theory, we could navigate around the entire filesystem, downloading any file we want, that PHP has access to. So the URI ?file=../../../config/license.key would allow a malicious user to download our site’s license key file!

To address this issue, it’s best to ensure that the file path is contained within its root directory. In other words, we need to ensure that the file path cannot be outside of the @webroot/assets/images/ directory. Fortunately, we can use the handy ensurePathIsContained method provided by Craft’s Path helper class to do this.

public function actionDownload(): Response
{
    $file = Craft::$app->request->getRequiredParam('file');

    // Throws an exception if the file path escapes its root directory.
    if (!Path::ensurePathIsContained($file)) {
        throw new BadRequestHttpException();
    }

    $filePath = Craft::getAlias('@webroot/assets/images/' . $file);

    return Craft::$app->response->sendFile($filePath);
}

Conclusion #

To quote the Highlander, there can only be one”, conclusion that is. Be as restrictive and explicit as possible with your plugin’s API and never ever trust user input. If you’re unsure as to whether your plugins are vulnerable to such attacks then hire someone to perform a security audit – you’ll not only gain peace of mind, you’ll also learn an invaluable lesson.

Sword