Saleor: Sensitive data exposed via Graphql

[timuric]

By default Saleor exposes sensitive business information like:

ProductVariant {
quantity: Int!
quantityAllocated: Int!
costPrice: Money
stockQuantity: Int!
margin: Int
quantityOrdered: Int
revenue(...): TaxedMoney
}

I hope nobody is running Saleor out there with theses fields exposed to public.

[maarcingebala]

Thanks for reporting. Some of these fields are used by the management dashboard and should be available only to authenticated users with proper permissions. I see that some of them are missing those permissions checks. They will remain in the public schema, but shouldn't be accessible by unauthorized users.

[timuric]

@maarcingebala I think it would be important to inform current shop owners about this.
Also looks like it is a design flaw since there is no field "inStock" for variants.
At the moment only way the storefront can tell if something is in stock is by reading the public stock property.

This is quite a serious issue for anyone running Saleor in production.

[maarcingebala]

Fix has been merged to master and released in Saleor 2.3.1 version.

[timuric]

Looks like #3773 doesn't cover the stock quantity

[maarcingebala]

I don't consider stock quantity to be as sensitive as financial data, I think it depends more on the use-case. Also, it seems that this field is currently used by Storefront 2.0. I'd like to discuss if we should restrict this field to privileged users or expose a boolean flag inStock instead. Another idea is to limit the maximal value that we show to unauthorized users to some arbitrary value.

[timuric]

I don't consider stock quantity to be as sensitive as financial data, I think it depends more on the use-case.

@maarcingebala If you multiply stock by price you can easily track the revenue. It is almost the same as exposing revenue. The only noise in data would be stock transfers, which is also not hard to account for.

Another idea is to limit the maximal value that we show to unauthorized users to some arbitrary value.

That would be nice, since it will allow to create "low in stock" notifications. It would be great to be able to adjust the threshold.

[jxltom]

I think revenue = (quantity - stock) * price? Maybe Saleor should limit the access to total quantity of a product variant?

[maarcingebala]

I agree that quantity and quantityAllocated should be private, but it's not exactly the same as exposing revenue. Allocated quantity tells how many items are currently allocated in open orders, but doesn't tell you anything about past orders. You would have to regularly fetch quantities to track that but still - the impact is not that heavy as directly exposing revenue.

[timuric]

@maarcingebala would be good to notify existing store owners about the issue so they can temporarily patch it. Some people might get fired for a data leak like that :)

[maarcingebala]

I'll mention that on Gitter and on Spectrum. This will be also mentioned in the incoming release notes on our blog.

[maarcingebala]

Oh, and we're waiting to get the CVE vulnerability ID for that bug.