Here is the original message as written by agoradesign. This was a security issue and it has been accepted by the commerce_rma team. For tracking purposes, this security issue has been ported to a public issue.

I've accidentially found out, that any user has access to view any commerce_return entity - at least the total value (e.g. for anonymous user that don't have access to line items at all - otherwise you'll see the full return entity including its line items!)

You can see this vulnerability by:
1. Enabling the module
2. Create a new return for any existing order.
3. Note the user ID belonging to the order/return and the return_id of the newly created commerce_return enttiy.
4. As a different user (or guest user) with at max. "View own [BUNDLE NAME] returns" permission (you don't need any return related permission at all) go to user/[USER_ID/returns/[RETURN_ID] with the values noted in step 3 (You can even enter 1 as uid for the super user)
5. Instead of getting a HTTP 403, you'll have view access to the foreign commerce_return entity! As noted above, you'll either see the full return info incl line items, of at least see the total sum, if you aren't allowed so view the line items at all.

I've already found the root of the problem. See the patch attached to this report.

Here's the explanation behind the problem:

  • commerce_rma defines its return entity in a submodule (commerce_return)
  • commerce_return handles many things the same way like commerce_order and relies on the generic commerce_entity_access() function of Drupal Commerce. Everything's fine so far...
  • The big difference here is, that the actual menu hook to view the return is implemented directly in commerce_rma module.
  • commerce_rma defines its menu hook as 'user/%user/returns/%commerce_return', leading to get any user object loaded as access argument that is entered in the URL.
  • Its access callback commerce_rma_access() delegates to commerce_return_access(), setting the operation, the return entity AND the user object as loaded as access argument. And this is the big problem!!!
  • Further, the access checks are done based on the wrong user object. In order to work correctly, the $account parameter must be left empty, so that in further calls the commerce_entity_access() loads the active user object and checks its access rights on the commerce_return entity instead.
CommentFileSizeAuthor
#1 commerce_rma-fix_bypass_view_access.patch509 bytesjkuma
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jkuma’s picture

jkuma’s picture

Status: Patch (to be ported) » Fixed

The patch has been pushed to dev branch.

Unfortunately, I made an error within commit message, the issue number is the one from the security post.

agoradesign is the original author of the patch and credit has been given to him.

@agoradesign thank you for your hard work on many commerce_rma issues!

agoradesign’s picture

My pleasure! It's always a great feeling to give something back to the community. The great teamwork is one of the key success factors of Drupal :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.