Possible integrity issue on jsonapi endpoints

Created on 28 June 2023, about 1 year ago

Problem/Motivation

We have implemented commerce_api and have our frontend submit info via the commerce_api. For instance the checkout call, where we submit billing/shipping profile information.
We noticed that you can add uid information, which results in being able to create profiles for a different user than yourself. We fixed this by adding an entity_update hook and setting the profile uid to always match the current_user (except for people having special permissions, so you can still use the cms to edit profiles/orders.

Steps to reproduce

curl 'http://[HOSTNAME]/jsonapi/checkout/9cec9580-9049-415d-ad2f-5e3ff92e37ea' -X PATCH -H 'Accept: application/vnd.api+json' -H 'Content-Type: application/vnd.api+json' -H 'Commerce-Cart-Token: [TOKEN]' -H 'Cookie: [SESSION]' --data-raw '{"data":{"type":"order--default","id":"9cec9580-9049-415d-ad2f-5e3ff92e37ea","attributes":{"email":"test@example.com","field_invoice_email":"test@example.com","field_customer_reference":"backoffice","billing_information":{"address":{"country_code":"NL","locality":"backoffice","postal_code":"backoffice","address_line1":"backoffice","address_line2":"backoffice","given_name":"test","family_name":"test","organization":"organization"},"field_first_name":"test","field_last_name":"test","field_telephone":"1234567890","uid":1 },"shipping_method":"1--default","shipping_information":{"address":{"country_code":"NL","locality":"locality","postal_code":"postal_code","address_line1":"address","address_line2":"address2","given_name":"test","family_name":"test","organization":"organization"},"field_first_name":"test","field_last_name":"test","field_telephone":"1234567890", "uid":1}}}}'
Note: there are some custom fields in this call, so either add those field to the profile, or remove them from the request. We also use the a order type 'default'

Proposed resolution

Do proper access check based on persmissons, although we can't seem to find a "add/edit own profile" permission

Expected results

We find it normal that we don't need to send the uid, and that it automatically will be set on the current user.

🐛 Bug report
Status

Active

Component

Code

Created by

🇳🇱Netherlands Roderik de Langen

Live updates comments and jobs are added and updated live.
  • Security

    It is used for security vulnerabilities which do not need a security advisory. For example, security issues in projects which do not have security advisory coverage, or forward-porting a change already disclosed in a security advisory. See Drupal’s security advisory policy for details. Be careful publicly disclosing security vulnerabilities! Use the “Report a security vulnerability” link in the project page’s sidebar. See how to report a security issue for details.

Sign in to follow issues

Comments & Activities

  • Issue created by @Roderik de Langen
  • 🇮🇱Israel jsacksick

    First of all, if there is a security issue, it shouldn't be reported publicly... Also...

        // Exclude keys, revision keys, and other base fields.
        $fields = array_filter($fields, function ($item) {
          return !($item instanceof BaseFieldDefinition);
        });
    

    This code from OrderProfile::propertyDefinitions() should exclude the uid field since it's a base field... So I'm surprised.

  • 🇳🇱Netherlands Roderik de Langen

    @jsacksick I was wondering about a public issue, but since this project is not covered by the security advisory it won't let me create private issues :( Apologies if I did had to submit it differently

    Also glad you have a thought already.

Production build 0.69.0 2024