From 840447e79269dd61d11fb4df8c20a7404e45245d Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Fri, 14 Aug 2026 08:54:56 +0200 Subject: [PATCH] Fix vendor product IDORs and consolidate ownership checks Two confirmed IDOR bugs in Grand.Web.Vendor product sub-resource mutations, plus a consolidation pass on how vendor ownership is checked across the area: - BundleProductModel field mismatch: Update/DeleteBundleProductModel loaded and mutated the parent product via ProductBundleId, but the global IProductValidVendor filter only validated ProductId (the bundled component). A vendor could pass their own product as ProductId to pass validation while mutating another vendor's ProductBundleId. Fixed with an explicit ownership check on the loaded parent. - ProductRelatedValidVendor OR-bypass: accepted ownership of either ProductId1 or ProductId2, but RelatedProductUpdate/Delete and SimilarProductUpdate/Delete only ever mutate ProductId1's mapping list. An attacker owning any product could supply it as ProductId2 to pass validation while editing someone else's ProductId1. Fixed by requiring ownership of ProductId1 only. - Consolidated ~50 inline `entity.VendorId != CurrentVendor.Id` checks (including a duplicate of HasAccessToProduct living as a private method on ProductController) onto the existing IWorkContext.HasAccessToX(entity) extensions in Extensions/HasAccess.cs, and added HasAccessToVendorReview / HasAccessToMerchandiseReturn to cover the two controllers that had no shared helper at all. Checking the loaded entity right before mutation (instead of a request DTO field) is what makes the first two bugs structurally impossible to reintroduce. - Documented the pattern and the failure mode with XML doc comments on HasAccess.cs and IProductValidVendor.cs. No test project exists yet for Grand.Web.Vendor; verified via `dotnet build src/Web/Grand.Web.Vendor/Grand.Web.Vendor.csproj`. Co-Authored-By: Claude Sonnet 5 --- .../MerchandiseReturnController.cs | 16 +++--- .../Controllers/ProductController.cs | 54 +++++++++---------- .../Controllers/ReportsController.cs | 3 +- .../Controllers/VendorReviewController.cs | 7 +-- .../Grand.Web.Vendor/Extensions/HasAccess.cs | 23 ++++++++ .../Models/Catalog/IProductValidVendor.cs | 20 +++++++ .../Services/OrderViewModelService.cs | 2 +- .../Services/ProductViewModelService.cs | 28 ++++++---- .../Services/ShipmentViewModelService.cs | 2 +- .../Services/VendorReviewViewModelService.cs | 5 +- .../Validators/Catalog/ProductValidVendor.cs | 9 ++-- 11 files changed, 113 insertions(+), 56 deletions(-) diff --git a/src/Web/Grand.Web.Vendor/Controllers/MerchandiseReturnController.cs b/src/Web/Grand.Web.Vendor/Controllers/MerchandiseReturnController.cs index e02f4db27..746210c81 100644 --- a/src/Web/Grand.Web.Vendor/Controllers/MerchandiseReturnController.cs +++ b/src/Web/Grand.Web.Vendor/Controllers/MerchandiseReturnController.cs @@ -77,7 +77,7 @@ await _merchandiseReturnViewModelService.PrepareMerchandiseReturnModel(model, co public async Task GoToId(MerchandiseReturnListModel model) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(model.GoDirectlyToId); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) //not found return RedirectToAction("List", "MerchandiseReturn"); @@ -89,7 +89,7 @@ public async Task GoToId(MerchandiseReturnListModel model) public async Task ProductsForMerchandiseReturn(string merchandiseReturnId) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(merchandiseReturnId); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) return ErrorForKendoGridJson("Merchandise return not found"); var items = await _merchandiseReturnViewModelService.PrepareMerchandiseReturnItemModel(merchandiseReturnId); @@ -106,7 +106,7 @@ public async Task ProductsForMerchandiseReturn(string merchandise public async Task Edit(string id) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(id); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) //No merchandise return found with the specified id return RedirectToAction("List"); @@ -125,7 +125,7 @@ [FromServices] OrderSettings orderSettings ) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(model.Id); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) //No merchandise return found with the specified id return RedirectToAction("List"); @@ -157,7 +157,7 @@ await _merchandiseReturnViewModelService.UpdateMerchandiseReturnModel(merchandis public async Task Delete(string id) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(id); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) //No merchandise return found with the specified id return RedirectToAction("List"); @@ -181,7 +181,7 @@ public async Task Delete(string id) public async Task MerchandiseReturnNotesSelect(string merchandiseReturnId) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(merchandiseReturnId); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) throw new ArgumentException("No merchandise return found with the specified id"); //merchandise return notes @@ -199,7 +199,7 @@ public async Task MerchandiseReturnNoteAdd(string merchandiseRetu string message) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(merchandiseReturnId); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) return Json(new { Result = false }); await _merchandiseReturnViewModelService.InsertMerchandiseReturnNote(merchandiseReturn, displayToCustomer, @@ -213,7 +213,7 @@ await _merchandiseReturnViewModelService.InsertMerchandiseReturnNote(merchandise public async Task MerchandiseReturnNoteDelete(string id, string merchandiseReturnId) { var merchandiseReturn = await _merchandiseReturnService.GetMerchandiseReturnById(merchandiseReturnId); - if (merchandiseReturn == null || merchandiseReturn.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (merchandiseReturn == null || !_contextAccessor.WorkContext.HasAccessToMerchandiseReturn(merchandiseReturn)) throw new ArgumentException("No merchandise return found with the specified id"); await _merchandiseReturnViewModelService.DeleteMerchandiseReturnNote(merchandiseReturn, id); diff --git a/src/Web/Grand.Web.Vendor/Controllers/ProductController.cs b/src/Web/Grand.Web.Vendor/Controllers/ProductController.cs index c23f36314..75a5109f8 100644 --- a/src/Web/Grand.Web.Vendor/Controllers/ProductController.cs +++ b/src/Web/Grand.Web.Vendor/Controllers/ProductController.cs @@ -83,7 +83,7 @@ public ProductController( if (product == null) return Task.FromResult((false, "Product not exists")); //a vendor should have access only to his products - return product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id + return !_contextAccessor.WorkContext.HasAccessToProduct(product) ? Task.FromResult((false, "This is not your product")) : Task.FromResult<(bool allow, string message)>((true, null)); } @@ -163,7 +163,7 @@ public async Task Create(ProductModel model, bool continueEditing public async Task Edit(string id) { var product = await _productService.GetProductById(id, true); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) //No product found with the specified id return RedirectToAction("List"); @@ -191,7 +191,7 @@ await AddLocales(_languageService, model.Locales, (locale, languageId) => public async Task Edit(ProductModel model, bool continueEditing) { var product = await _productService.GetProductById(model.Id, true); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) //No product found with the specified id return RedirectToAction("List"); @@ -228,7 +228,7 @@ public async Task Edit(ProductModel model, bool continueEditing) public async Task Delete(string id) { var product = await _productService.GetProductById(id, true); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) //No product found with the specified id return RedirectToAction("List"); @@ -262,7 +262,7 @@ public async Task CopyProduct(ProductModel model, { var originalProduct = await _productService.GetProductById(copyModel.Id, true); //a vendor should have access only to his products - if (originalProduct == null || originalProduct.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (originalProduct == null || !_contextAccessor.WorkContext.HasAccessToProduct(originalProduct)) return RedirectToAction("List"); var newProduct = await copyProductService.CopyProduct(originalProduct, @@ -323,7 +323,7 @@ public async Task LoadProductFriendlyNames(string productIds) var products = await _productService.GetProductsByIds(Enumerable.ToArray(rangeArray), true); for (var i = 0; i <= products.Count - 1; i++) { - if (products[i].VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (!_contextAccessor.WorkContext.HasAccessToProduct(products[i])) continue; result += products[i].Name; if (i != products.Count - 1) @@ -1003,7 +1003,7 @@ public async Task AssociatedProductUpdate(ProductModel.Associated if (ModelState.IsValid) { var associatedProduct = await _productService.GetProductById(model.Id); - if (associatedProduct == null || associatedProduct.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (associatedProduct == null || !_contextAccessor.WorkContext.HasAccessToProduct(associatedProduct)) throw new ArgumentException("No associated product found with the specified id"); associatedProduct.DisplayOrder = model.DisplayOrder; @@ -1022,7 +1022,7 @@ public async Task AssociatedProductDelete(ProductModel.Associated if (ModelState.IsValid) { var product = await _productService.GetProductById(model.Id); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No associated product found with the specified id"); await _productViewModelService.DeleteAssociatedProduct(product); @@ -1102,7 +1102,7 @@ public async Task ProductPictureAdd( var product = await _productService.GetProductById(objectId); //a vendor should have access only to his products - if (product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (!_contextAccessor.WorkContext.HasAccessToProduct(product)) return Json(new { success = false, message = "Access denied - vendor permissions" @@ -1438,7 +1438,7 @@ public async Task ExportExcelSelected(string selectedIds, } //a vendor should have access only to his products - products = products.Where(p => p.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id).ToList(); + products = products.Where(p => _contextAccessor.WorkContext.HasAccessToProduct(p)).ToList(); var bytes = await exportManager.Export(products); return File(bytes, "text/xls", "products.xlsx"); @@ -1664,7 +1664,7 @@ public async Task TierPriceEditPopup(string id, string productId) return Content("Empty tier price"); //a vendor should have access only to his products - if (product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (!_contextAccessor.WorkContext.HasAccessToProduct(product)) return Content("This is not your product"); var model = tierPrice.ToModel(_dateTimeService); @@ -1795,7 +1795,7 @@ public async Task ProductAttributeMappingDelete(string id, string throw new ArgumentException("No product attribute mapping found with the specified id"); //a vendor should have access only to his products - if (product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (!_contextAccessor.WorkContext.HasAccessToProduct(product)) return Content("This is not your product"); await productAttributeService.DeleteProductAttributeMapping(productAttributeMapping, product.Id); @@ -1901,7 +1901,7 @@ public async Task EditAttributeValues(string productAttributeMapp [FromServices] IProductAttributeService productAttributeService) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var productAttributeMapping = @@ -2028,7 +2028,7 @@ public async Task ProductAttributeValueEditPopup(string productId ProductModel.ProductAttributeValueModel model) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var pav = product.ProductAttributeMappings.FirstOrDefault(x => x.Id == model.ProductAttributeMappingId) @@ -2055,7 +2055,7 @@ public async Task ProductAttributeValueDelete(string id, string p [FromServices] IProductAttributeService productAttributeService) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var pav = product.ProductAttributeMappings.FirstOrDefault(x => x.Id == pam)?.ProductAttributeValues @@ -2098,7 +2098,7 @@ public async Task AssociateProductToAttributeValuePopup( ProductModel.ProductAttributeValueModel.AssociateProductToAttributeValueModel model) { var associatedProduct = await _productService.GetProductById(model.AssociatedToProductId); - if (associatedProduct == null || associatedProduct.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (associatedProduct == null || !_contextAccessor.WorkContext.HasAccessToProduct(associatedProduct)) return Content("Cannot load a product"); return Content(""); @@ -2132,7 +2132,7 @@ public async Task ProductAttributeCombinationDelete(string id, st [FromServices] IProductAttributeService productAttributeService) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var combination = product.ProductAttributeCombinations.FirstOrDefault(x => x.Id == id); @@ -2173,7 +2173,7 @@ public async Task AttributeCombinationPopup(string productId, ProductAttributeCombinationModel model) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) //No product found with the specified id return RedirectToAction("List", "Product"); @@ -2192,7 +2192,7 @@ public async Task AttributeCombinationPopup(string productId, public async Task GenerateAllAttributeCombinations(string productId) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); await _productViewModelService.GenerateAllAttributeCombinations(product); @@ -2205,7 +2205,7 @@ public async Task GenerateAllAttributeCombinations(string product public async Task ClearAllAttributeCombinations(string productId) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); if (ModelState.IsValid) @@ -2296,7 +2296,7 @@ public async Task ProductAttributeCombinationTierPriceDelete(stri string productAttributeCombinationId, string id) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var combination = @@ -2355,7 +2355,7 @@ await _productReservationService.GetProductReservationsByProductId(productId, nu public async Task GenerateCalendar(ProductModel.GenerateCalendarModel model) { var product = await _productService.GetProductById(model.ProductId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var reservations = @@ -2486,7 +2486,7 @@ await _productReservationService.InsertProductReservation(new ProductReservation public async Task ClearCalendar(string productId) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var toDelete = await _productReservationService.GetProductReservationsByProductId(productId, true, null); @@ -2499,7 +2499,7 @@ public async Task ClearCalendar(string productId) public async Task ClearOld(string productId) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var toDelete = @@ -2515,7 +2515,7 @@ public async Task ClearOld(string productId) public async Task ProductReservationDelete(ProductModel.ReservationModel model) { var product = await _productService.GetProductById(model.ProductId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var toDelete = await _productReservationService.GetProductReservation(model.ReservationId); @@ -2542,7 +2542,7 @@ public async Task ProductReservationDelete(ProductModel.Reservati public async Task ListBids(DataSourceRequest command, string productId) { var product = await _productService.GetProductById(productId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var (bidModels, totalCount) = @@ -2559,7 +2559,7 @@ public async Task ListBids(DataSourceRequest command, string prod public async Task BidDelete(ProductModel.BidModel model) { var product = await _productService.GetProductById(model.ProductId); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) throw new ArgumentException("No product found with the specified id"); var toDelete = await _auctionService.GetBid(model.BidId); diff --git a/src/Web/Grand.Web.Vendor/Controllers/ReportsController.cs b/src/Web/Grand.Web.Vendor/Controllers/ReportsController.cs index 50fa108a1..5bc4b57b8 100644 --- a/src/Web/Grand.Web.Vendor/Controllers/ReportsController.cs +++ b/src/Web/Grand.Web.Vendor/Controllers/ReportsController.cs @@ -12,6 +12,7 @@ using Grand.Web.Common.Extensions; using Grand.Web.Common.Localization; using Grand.Web.Common.Security.Authorization; +using Grand.Web.Vendor.Extensions; using Grand.Web.Vendor.Models.Report; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Rendering; @@ -176,7 +177,7 @@ public async Task BestsellersReportList(DataSourceRequest command m.ProductName = product.Name; if (_contextAccessor.WorkContext.CurrentVendor != null) { - if (product?.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id) + if (product != null && _contextAccessor.WorkContext.HasAccessToProduct(product)) result.Add(m); } else diff --git a/src/Web/Grand.Web.Vendor/Controllers/VendorReviewController.cs b/src/Web/Grand.Web.Vendor/Controllers/VendorReviewController.cs index a26197396..b27ea0a9b 100644 --- a/src/Web/Grand.Web.Vendor/Controllers/VendorReviewController.cs +++ b/src/Web/Grand.Web.Vendor/Controllers/VendorReviewController.cs @@ -5,6 +5,7 @@ using Grand.Web.Common.DataSource; using Grand.Web.Common.Filters; using Grand.Web.Common.Security.Authorization; +using Grand.Web.Vendor.Extensions; using Grand.Web.Vendor.Interfaces; using Grand.Web.Vendor.Models.VendorReview; using Microsoft.AspNetCore.Mvc; @@ -73,7 +74,7 @@ public async Task Edit(string id) { var vendorReview = await _vendorService.GetVendorReviewById(id); - if (vendorReview == null || vendorReview.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (vendorReview == null || !_contextAccessor.WorkContext.HasAccessToVendorReview(vendorReview)) //No vendor review found with the specified id return RedirectToAction("List"); @@ -88,7 +89,7 @@ public async Task Edit(string id) public async Task Edit(VendorReviewModel model, bool continueEditing) { var vendorReview = await _vendorService.GetVendorReviewById(model.Id); - if (vendorReview == null || vendorReview.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (vendorReview == null || !_contextAccessor.WorkContext.HasAccessToVendorReview(vendorReview)) //No vendor review found with the specified id return RedirectToAction("List"); @@ -112,7 +113,7 @@ public async Task Edit(VendorReviewModel model, bool continueEdit public async Task Delete(string id) { var vendorReview = await _vendorService.GetVendorReviewById(id); - if (vendorReview == null || vendorReview.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (vendorReview == null || !_contextAccessor.WorkContext.HasAccessToVendorReview(vendorReview)) //No vendor review found with the specified id return RedirectToAction("List"); diff --git a/src/Web/Grand.Web.Vendor/Extensions/HasAccess.cs b/src/Web/Grand.Web.Vendor/Extensions/HasAccess.cs index 290997687..e149d85be 100644 --- a/src/Web/Grand.Web.Vendor/Extensions/HasAccess.cs +++ b/src/Web/Grand.Web.Vendor/Extensions/HasAccess.cs @@ -1,10 +1,19 @@ using Grand.Domain.Catalog; using Grand.Domain.Orders; using Grand.Domain.Shipping; +using Grand.Domain.Vendors; using Grand.Infrastructure; namespace Grand.Web.Vendor.Extensions; +/// +/// Vendor tenant-isolation checks. This is the one place ownership of a domain entity is decided for +/// the Vendor area - always check the loaded entity you are about to read/mutate through one of these +/// methods rather than comparing `entity.VendorId` inline. Checking the entity itself (instead of an +/// incoming request/DTO field) guarantees the object you authorized is the same object you act on - +/// see IProductValidVendor in Grand.Web.Vendor.Models.Catalog for the class of bug that arises when +/// those two are allowed to drift apart. +/// public static class HasAccess { public static bool HasAccessToProduct(this IWorkContext workContext, Product product) @@ -35,4 +44,18 @@ public static bool HasAccessToShipment(this IWorkContext workContext, Shipment s return shipment.VendorId == workContext.CurrentVendor.Id; } + + public static bool HasAccessToVendorReview(this IWorkContext workContext, VendorReview vendorReview) + { + ArgumentNullException.ThrowIfNull(vendorReview); + + return vendorReview.VendorId == workContext.CurrentVendor.Id; + } + + public static bool HasAccessToMerchandiseReturn(this IWorkContext workContext, MerchandiseReturn merchandiseReturn) + { + ArgumentNullException.ThrowIfNull(merchandiseReturn); + + return merchandiseReturn.VendorId == workContext.CurrentVendor.Id; + } } \ No newline at end of file diff --git a/src/Web/Grand.Web.Vendor/Models/Catalog/IProductValidVendor.cs b/src/Web/Grand.Web.Vendor/Models/Catalog/IProductValidVendor.cs index 0445319bc..73b8c098a 100644 --- a/src/Web/Grand.Web.Vendor/Models/Catalog/IProductValidVendor.cs +++ b/src/Web/Grand.Web.Vendor/Models/Catalog/IProductValidVendor.cs @@ -1,10 +1,30 @@ namespace Grand.Web.Vendor.Models.Catalog; +/// +/// Implement on any Vendor-area POST model that carries a product id. The global +/// resolves an +/// IValidator<IProductValidVendor> for every such model and rejects the request unless +/// belongs to the current vendor - see +/// Grand.Web.Vendor.Validators.Catalog.ProductValidVendor. +/// IMPORTANT: if the model carries more than one product-id-shaped field (e.g. an owning/parent id +/// plus a referenced/component id), make sure is bound to whichever id the +/// action actually mutates. A mismatch silently authorizes the wrong product - see +/// ProductModel.BundleProductModel (ProductId vs ProductBundleId) for a bug of this exact shape that +/// was found and fixed. +/// public interface IProductValidVendor { public string ProductId { get; set; } } +/// +/// Implement on Vendor-area POST models that relate two products (e.g. related/similar products). +/// The paired validator (ProductRelatedValidVendor) requires ownership of +/// only, because the consuming actions only ever read/mutate ProductId1's mapping list. Do not widen +/// this to accept ownership of either id ("OR") unless the action is also changed to only ever +/// mutate whichever product is actually owned - an OR check let an attacker satisfy validation via +/// ProductId2 while mutating a ProductId1 they don't own. +/// public interface IProductRelatedValidVendor { public string ProductId1 { get; set; } diff --git a/src/Web/Grand.Web.Vendor/Services/OrderViewModelService.cs b/src/Web/Grand.Web.Vendor/Services/OrderViewModelService.cs index 9e0af66f4..f1b81aa3f 100644 --- a/src/Web/Grand.Web.Vendor/Services/OrderViewModelService.cs +++ b/src/Web/Grand.Web.Vendor/Services/OrderViewModelService.cs @@ -427,7 +427,7 @@ await _addressAttributeParser.FormatAttributes(_contextAccessor.WorkContext.Work model.CheckoutAttributeInfo = order.CheckoutAttributeDescription; var hasDownloadableItems = false; var products = order.OrderItems - .Where(orderItem => orderItem.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id) + .Where(orderItem => _contextAccessor.WorkContext.HasAccessToOrderItem(orderItem)) .ToList(); foreach (var orderItem in products) diff --git a/src/Web/Grand.Web.Vendor/Services/ProductViewModelService.cs b/src/Web/Grand.Web.Vendor/Services/ProductViewModelService.cs index 6d277a2b2..a190db9a4 100644 --- a/src/Web/Grand.Web.Vendor/Services/ProductViewModelService.cs +++ b/src/Web/Grand.Web.Vendor/Services/ProductViewModelService.cs @@ -750,7 +750,7 @@ public virtual async Task DeleteSelected(IEnumerable selectedIds) { var product = products[i]; //a vendor should have access only to his products - if (product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (!_contextAccessor.WorkContext.HasAccessToProduct(product)) continue; await DeleteProduct(product); @@ -898,7 +898,7 @@ public virtual async Task InsertRelatedProductModel(ProductModel.AddRelatedProdu foreach (var id in model.SelectedProductIds) { var product = await _productService.GetProductById(id); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) continue; var existingRelatedProducts = productId1.RelatedProducts; if (model.ProductId == id) continue; @@ -943,7 +943,7 @@ public virtual async Task InsertSimilarProductModel(ProductModel.AddSimilarProdu foreach (var id in model.SelectedProductIds) { var product = await _productService.GetProductById(id); - if (product != null && productId1.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id) + if (product != null && _contextAccessor.WorkContext.HasAccessToProduct(productId1)) { var existingSimilarProducts = productId1.SimilarProducts; if (model.ProductId != id) @@ -991,7 +991,7 @@ public virtual async Task InsertBundleProductModel(ProductModel.AddBundleProduct foreach (var id in model.SelectedProductIds) { var product = await _productService.GetProductById(id); - if (product != null && productId1.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id) + if (product != null && _contextAccessor.WorkContext.HasAccessToProduct(productId1)) { var existingBundleProducts = productId1.BundleProducts; if (model.ProductId != id) @@ -1012,6 +1012,11 @@ public virtual async Task InsertBundleProductModel(ProductModel.AddBundleProduct public virtual async Task UpdateBundleProductModel(ProductModel.BundleProductModel model) { var product = await _productService.GetProductById(model.ProductBundleId, true); + //the IProductValidVendor filter validates model.ProductId (the bundled item), not + //model.ProductBundleId (the product actually mutated below) - check it explicitly + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) + throw new ArgumentException("No product found with the specified id"); + var bundleProduct = product.BundleProducts.FirstOrDefault(x => x.Id == model.Id); if (bundleProduct == null) throw new ArgumentException("No bundle product found with the specified id"); @@ -1025,6 +1030,11 @@ public virtual async Task UpdateBundleProductModel(ProductModel.BundleProductMod public virtual async Task DeleteBundleProductModel(ProductModel.BundleProductModel model) { var product = await _productService.GetProductById(model.ProductBundleId, true); + //the IProductValidVendor filter validates model.ProductId (the bundled item), not + //model.ProductBundleId (the product actually mutated below) - check it explicitly + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) + throw new ArgumentException("No product found with the specified id"); + var bundleProduct = product.BundleProducts.FirstOrDefault(x => x.Id == model.Id); if (bundleProduct == null) throw new ArgumentException("No bundle product found with the specified id"); @@ -1038,7 +1048,7 @@ public virtual async Task InsertCrossSellProductModel(ProductModel.AddCrossSellP foreach (var id in model.SelectedProductIds) { var product = await _productService.GetProductById(id); - if (product != null && product.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id && + if (product != null && _contextAccessor.WorkContext.HasAccessToProduct(product) && crossSellProduct.CrossSellProduct.All(x => x != id)) if (model.ProductId != id) await _productService.InsertCrossSellProduct( @@ -1064,7 +1074,7 @@ public virtual async Task InsertRecommendedProductModel(ProductModel.AddRecommen foreach (var id in model.SelectedProductIds) { var product = await _productService.GetProductById(id); - if (product != null && product.VendorId == _contextAccessor.WorkContext.CurrentVendor.Id) + if (product != null && _contextAccessor.WorkContext.HasAccessToProduct(product)) if (mainproduct.RecommendedProduct.All(x => x != id)) if (model.ProductId != id) await _productService.InsertRecommendedProduct(model.ProductId, id); @@ -1081,7 +1091,7 @@ public virtual async Task InsertAssociatedProductModel(ProductModel.AddAssociate foreach (var id in model.SelectedProductIds) { var product = await _productService.GetProductById(id); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) continue; product.ParentGroupedProductId = model.ProductId; await _productService.UpdateAssociatedProduct(product); } @@ -1182,7 +1192,7 @@ public virtual async Task UpdateBulkEdit(IEnumerable produ { //update var product = await _productService.GetProductById(pModel.Id, true); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) continue; var prevStockQuantity = _stockQuantityService.GetTotalStockQuantity(product, total: true); @@ -1212,7 +1222,7 @@ public virtual async Task DeleteBulkEdit(IEnumerable produ { //delete var product = await _productService.GetProductById(pModel.Id, true); - if (product == null || product.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (product == null || !_contextAccessor.WorkContext.HasAccessToProduct(product)) continue; await _productService.DeleteProduct(product); } diff --git a/src/Web/Grand.Web.Vendor/Services/ShipmentViewModelService.cs b/src/Web/Grand.Web.Vendor/Services/ShipmentViewModelService.cs index 98c1fffef..de40c522c 100644 --- a/src/Web/Grand.Web.Vendor/Services/ShipmentViewModelService.cs +++ b/src/Web/Grand.Web.Vendor/Services/ShipmentViewModelService.cs @@ -104,7 +104,7 @@ public virtual async Task PrepareShipmentModel(Shipment shipment, if (orderItem == null) continue; - if (orderItem.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) + if (!_contextAccessor.WorkContext.HasAccessToOrderItem(orderItem)) continue; //quantities diff --git a/src/Web/Grand.Web.Vendor/Services/VendorReviewViewModelService.cs b/src/Web/Grand.Web.Vendor/Services/VendorReviewViewModelService.cs index 03ed96498..215f74017 100644 --- a/src/Web/Grand.Web.Vendor/Services/VendorReviewViewModelService.cs +++ b/src/Web/Grand.Web.Vendor/Services/VendorReviewViewModelService.cs @@ -5,6 +5,7 @@ using Grand.Domain.Vendors; using Grand.Infrastructure; using Grand.SharedKernel.Extensions; +using Grand.Web.Vendor.Extensions; using Grand.Web.Vendor.Interfaces; using Grand.Web.Vendor.Models.VendorReview; using Grand.Mediator; @@ -115,7 +116,7 @@ public virtual async Task ApproveVendorReviews(IEnumerable selectedIds) foreach (var id in selectedIds) { var vendorReview = await _vendorService.GetVendorReviewById(id); - if (vendorReview == null || vendorReview.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (vendorReview == null || !_contextAccessor.WorkContext.HasAccessToVendorReview(vendorReview)) continue; var previousIsApproved = vendorReview.IsApproved; vendorReview.IsApproved = true; @@ -133,7 +134,7 @@ public virtual async Task DisapproveVendorReviews(IEnumerable selectedId foreach (var id in selectedIds) { var vendorReview = await _vendorService.GetVendorReviewById(id); - if (vendorReview == null || vendorReview.VendorId != _contextAccessor.WorkContext.CurrentVendor.Id) continue; + if (vendorReview == null || !_contextAccessor.WorkContext.HasAccessToVendorReview(vendorReview)) continue; vendorReview.IsApproved = false; await _vendorService.UpdateVendorReview(vendorReview); diff --git a/src/Web/Grand.Web.Vendor/Validators/Catalog/ProductValidVendor.cs b/src/Web/Grand.Web.Vendor/Validators/Catalog/ProductValidVendor.cs index f9173e5c9..26f388870 100644 --- a/src/Web/Grand.Web.Vendor/Validators/Catalog/ProductValidVendor.cs +++ b/src/Web/Grand.Web.Vendor/Validators/Catalog/ProductValidVendor.cs @@ -32,12 +32,13 @@ public ProductRelatedValidVendor( { RuleFor(x => x).MustAsync(async (x, _, _) => { + //RelatedProductModel/SimilarProductModel actions only ever read and mutate ProductId1's + //mapping list, so ownership of ProductId1 is what must be enforced here. Accepting ownership + //of ProductId2 as an alternative (the previous "||") let a vendor who owns any product supply + //it as ProductId2 and edit/delete another vendor's ProductId1 mapping. var product1 = await productService.GetProductById(x.ProductId1); if (product1 == null) return true; - var product2 = await productService.GetProductById(x.ProductId2); - if (product2 == null) return true; - return product1.VendorId == contextAccessor.WorkContext.CurrentVendor.Id || - product2.VendorId == contextAccessor.WorkContext.CurrentVendor.Id; + return product1.VendorId == contextAccessor.WorkContext.CurrentVendor.Id; }).WithMessage(translationService.GetResource("Vendor.Catalog.Products.Permissions")); } } \ No newline at end of file