From e8f4c77372c14b46b3ac4b5ecd5dfe2518378895 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 5 Jul 2026 11:01:41 +0200 Subject: [PATCH 1/2] Fix cross-store IDOR in store-panel customer product price/personalization The store-panel CustomerController exposed four actions that mutated CustomerProductPrice / CustomerProduct entities by raw id without verifying the underlying customer belongs to the current store: UpdateProductPrice, DeleteProductPrice, UpdatePersonalizedProduct, DeletePersonalizedProduct. The AdminShared view-model service loads these entities by id and saves/deletes them with no store filter, so a store manager could update or delete another store customer's custom price or personalized products by supplying a foreign id. Resolve the entity, take its CustomerId and run it through the existing GetStoreCustomer() gate (store + registered check) before mutating - matching the pattern already used by UpdateCart/DeleteCart. No domain model or shared service changes. Co-Authored-By: Claude Fable 5 --- .../Controllers/CustomerController.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Web/Grand.Web.Store/Controllers/CustomerController.cs b/src/Web/Grand.Web.Store/Controllers/CustomerController.cs index 93ddccd0b..6715bb7c8 100644 --- a/src/Web/Grand.Web.Store/Controllers/CustomerController.cs +++ b/src/Web/Grand.Web.Store/Controllers/CustomerController.cs @@ -4,6 +4,7 @@ using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Customers; +using Grand.Business.Core.Interfaces.Marketing.Customers; using Grand.Business.Core.Interfaces.Messages; using Grand.Business.Core.Utilities.Customers; using Grand.Domain.Catalog; @@ -45,6 +46,7 @@ public CustomerController( ICustomerService customerService, ICustomerViewModelService customerViewModelService, ICustomerManagerService customerManagerService, + ICustomerProductService customerProductService, IProductReviewService productReviewService, IProductReviewViewModelService productReviewViewModelService, IProductViewModelService productViewModelService, @@ -62,6 +64,7 @@ public CustomerController( _customerService = customerService; _customerViewModelService = customerViewModelService; _customerManagerService = customerManagerService; + _customerProductService = customerProductService; _productReviewService = productReviewService; _productReviewViewModelService = productReviewViewModelService; _productViewModelService = productViewModelService; @@ -84,6 +87,7 @@ public CustomerController( private readonly ICustomerService _customerService; private readonly ICustomerViewModelService _customerViewModelService; private readonly ICustomerManagerService _customerManagerService; + private readonly ICustomerProductService _customerProductService; private readonly IProductReviewService _productReviewService; private readonly IProductReviewViewModelService _productReviewViewModelService; private readonly IProductViewModelService _productViewModelService; @@ -829,6 +833,10 @@ public async Task ProductAddPopup(string customerId, bool persona [PermissionAuthorizeAction(PermissionActionName.Edit)] public async Task UpdateProductPrice(CustomerModel.ProductPriceModel model) { + var productPrice = await _customerProductService.GetCustomerProductPriceById(model.Id); + if (productPrice == null || await GetStoreCustomer(productPrice.CustomerId) == null) + return new JsonResult(""); + await _customerViewModelService.UpdateProductPrice(model); return new JsonResult(""); } @@ -836,6 +844,10 @@ public async Task UpdateProductPrice(CustomerModel.ProductPriceMo [PermissionAuthorizeAction(PermissionActionName.Edit)] public async Task DeleteProductPrice(string id) { + var productPrice = await _customerProductService.GetCustomerProductPriceById(id); + if (productPrice == null || await GetStoreCustomer(productPrice.CustomerId) == null) + return new JsonResult(""); + await _customerViewModelService.DeleteProductPrice(id); return new JsonResult(""); } @@ -843,6 +855,10 @@ public async Task DeleteProductPrice(string id) [PermissionAuthorizeAction(PermissionActionName.Edit)] public async Task UpdatePersonalizedProduct(CustomerModel.ProductModel model) { + var customerProduct = await _customerProductService.GetCustomerProduct(model.Id); + if (customerProduct == null || await GetStoreCustomer(customerProduct.CustomerId) == null) + return new JsonResult(""); + await _customerViewModelService.UpdatePersonalizedProduct(model); return new JsonResult(""); } @@ -850,6 +866,10 @@ public async Task UpdatePersonalizedProduct(CustomerModel.Product [PermissionAuthorizeAction(PermissionActionName.Edit)] public async Task DeletePersonalizedProduct(string id) { + var customerProduct = await _customerProductService.GetCustomerProduct(id); + if (customerProduct == null || await GetStoreCustomer(customerProduct.CustomerId) == null) + return new JsonResult(""); + await _customerViewModelService.DeletePersonalizedProduct(id); return new JsonResult(""); } From 7912153b428cb04555d4118146149c9c85351109 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 5 Jul 2026 11:54:10 +0200 Subject: [PATCH 2/2] Address review: restrict actions to POST, fix test ctor, avoid TOCTOU - Add [HttpPost] to UpdateProductPrice/DeleteProductPrice/ UpdatePersonalizedProduct/DeletePersonalizedProduct so the [AutoValidateAntiforgeryToken] on BaseStoreController is enforced (the Kendo grids already call them via POST) - closes the CSRF gap Copilot flagged. - Delete the already-resolved entity via ICustomerProductService instead of re-loading by id in the view-model service, removing the redundant query and the TOCTOU exception on a concurrent delete. - Update CustomerControllerTests to supply the new ICustomerProductService constructor dependency (fixes the CI build break). Co-Authored-By: Claude Fable 5 --- .../Controllers/CustomerControllerTests.cs | 2 ++ .../Grand.Web.Store/Controllers/CustomerController.cs | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Tests/Grand.Web.Store.Tests/Controllers/CustomerControllerTests.cs b/src/Tests/Grand.Web.Store.Tests/Controllers/CustomerControllerTests.cs index 2a91c0b62..21f63d44a 100644 --- a/src/Tests/Grand.Web.Store.Tests/Controllers/CustomerControllerTests.cs +++ b/src/Tests/Grand.Web.Store.Tests/Controllers/CustomerControllerTests.cs @@ -3,6 +3,7 @@ using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Customers; +using Grand.Business.Core.Interfaces.Marketing.Customers; using Grand.Business.Core.Interfaces.Messages; using Grand.Domain.Customers; using Grand.Infrastructure; @@ -59,6 +60,7 @@ private CustomerController BuildController(bool perStoreEnabled) _customerServiceMock.Object, _customerViewModelServiceMock.Object, _customerManagerServiceMock.Object, + new Mock().Object, new Mock().Object, new Mock().Object, new Mock().Object, diff --git a/src/Web/Grand.Web.Store/Controllers/CustomerController.cs b/src/Web/Grand.Web.Store/Controllers/CustomerController.cs index 6715bb7c8..22797b69b 100644 --- a/src/Web/Grand.Web.Store/Controllers/CustomerController.cs +++ b/src/Web/Grand.Web.Store/Controllers/CustomerController.cs @@ -831,6 +831,7 @@ public async Task ProductAddPopup(string customerId, bool persona } [PermissionAuthorizeAction(PermissionActionName.Edit)] + [HttpPost] public async Task UpdateProductPrice(CustomerModel.ProductPriceModel model) { var productPrice = await _customerProductService.GetCustomerProductPriceById(model.Id); @@ -842,17 +843,20 @@ public async Task UpdateProductPrice(CustomerModel.ProductPriceMo } [PermissionAuthorizeAction(PermissionActionName.Edit)] + [HttpPost] public async Task DeleteProductPrice(string id) { var productPrice = await _customerProductService.GetCustomerProductPriceById(id); if (productPrice == null || await GetStoreCustomer(productPrice.CustomerId) == null) return new JsonResult(""); - await _customerViewModelService.DeleteProductPrice(id); + //delete the already-resolved entity to avoid a redundant reload / TOCTOU throw + await _customerProductService.DeleteCustomerProductPrice(productPrice); return new JsonResult(""); } [PermissionAuthorizeAction(PermissionActionName.Edit)] + [HttpPost] public async Task UpdatePersonalizedProduct(CustomerModel.ProductModel model) { var customerProduct = await _customerProductService.GetCustomerProduct(model.Id); @@ -864,13 +868,15 @@ public async Task UpdatePersonalizedProduct(CustomerModel.Product } [PermissionAuthorizeAction(PermissionActionName.Edit)] + [HttpPost] public async Task DeletePersonalizedProduct(string id) { var customerProduct = await _customerProductService.GetCustomerProduct(id); if (customerProduct == null || await GetStoreCustomer(customerProduct.CustomerId) == null) return new JsonResult(""); - await _customerViewModelService.DeletePersonalizedProduct(id); + //delete the already-resolved entity to avoid a redundant reload / TOCTOU throw + await _customerProductService.DeleteCustomerProduct(customerProduct); return new JsonResult(""); }