[FIX] Klaviyo_Reclaim: Update Cart Controller#353
[FIX] Klaviyo_Reclaim: Update Cart Controller#353tedsalmon wants to merge 2 commits intoklaviyo:masterfrom
Conversation
* Add support for unmasked quote IDs since logged in carts are not assigned a masked quote ID
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ->load($quoteId, 'masked_id'); | ||
| $quoteId = $quoteIdMask->getQuoteId(); | ||
| } | ||
| $quote = $this->quoteRepository->get($quoteId); |
There was a problem hiding this comment.
Bug: Unmasked quote IDs allow unauthorized cart access via enumeration
The new code accepts numeric (unmasked) quote IDs directly without verifying that the quote belongs to the current user. Since Magento quote IDs are simple auto-incrementing integers, an attacker could enumerate quote IDs (e.g., quote_id=1, quote_id=2, etc.) to load other users' carts into their session. Masked quote IDs exist specifically to prevent this IDOR vulnerability. The code lacks ownership verification when processing numeric IDs, allowing potential unauthorized access to any user's cart.
There was a problem hiding this comment.
Cursor is right, but if we added session validation, then reclaim would no longer work for users after their session expires. Perhaps a more meaningful fix can be added by the Klaviyo team by encrypting the quoe_id using the kx_identifier on their end.
Another possibility I thought about was forcing all quotes to have a masked_id generated at creation time, but I was not thrilled about modifying core functionality.
There was a problem hiding this comment.
I'm with Cursor as well. We will evaluate safer options.
There was a problem hiding this comment.
FWIW, my second commit adds a check to ensure the logged in customer owns the quote
There was a problem hiding this comment.
@tedsalmon thanks for submitting - i'll flag to the team to discuss after the holidays!
* Make sure that the customer owns the quote being loaded. This only applies to quotes for customers who are not logged in This is the commit message klaviyo#2:
tedsalmon
force-pushed
the
bugfix/support-unmasked-quote-ids
branch
from
94719d3 to
0191418
Compare
| ->load($quoteId, 'masked_id'); | ||
| $quoteId = $quoteIdMask->getQuoteId(); | ||
| } | ||
| $quote = $this->quoteRepository->get($quoteId); |
There was a problem hiding this comment.
I'm with Cursor as well. We will evaluate safer options.
|
@tedsalmon I'm having a hard time testing this, there arent any cases I'm seeing where a logged in user doesn't have a quote id that is formatted as |
3 participants
Description
Adds support for unmasked quote IDs since logged in carts are not assigned a masked quote ID.
Manual Testing Steps
Pre-Submission Checklist: