The maximum CFO offerCut is 50% of T

[ghost]

Description

S (offerCut) can only ever be 50% of T (Total).

If this is expected behavior, it is unclear that it is so. Even if it is so, it may cause the CFO to accidentally set offerCut to half of what they intend.

The comments to _computeOfferPrice make it sound as if S is expected to range from 0.00% to 100.00% of T, with a level of precision of basis points (hundredths of a percentile).

However, the formula at the top of OffersConfig.sol implies that S can only ever be 50% of T. But even if the formula implies that S can only ever be 50% of T, no such cap is ever explicitly stated in the description of expected behavior. Since Offers.sol is aimed at being a generic contract for any ERC721 token, it seems that it should allow for a CFO to set a fee anywhere between 0% and 100% of T.

Scenario

In the comments for _computeOfferPrice, expected behavior is that _offerCut will be "The percentage in basis points that will be taken by the CFO." This is the one parameter given to the function.

The percentage in basis points is described as being "values 0-10,000 map to 0%-100%".

This should allow for _offerCut to be any percentage between 0% and 100% of T, not simply between 0% and 50% of T.

However, _computeOfferPrice only returns a P (offerPrice) between 50% and 100% of T, leaving only 0% to 50% as S (cfoEarnings).

Impact

High: The math in _computeOfferPrice gives a CFO half of the revenue that they would expect. Additionally, Offers.sol is stated as being a generic contract that could be used by any ERC721 token, but other CFO's will not know that _computeOfferPrice is limited to 50%, meaning that they will receive half of the income that they expect, and won't have the flexibility to adjust the offerCut to anything from 50% to 100% of T.

Reproduction

(1) Have CFO call setOfferCut(10000).
(2) Since this is 10,000 basis points, that is 100%. According to the comments for _computeOfferPrice, we would expect S (offerCut) to encompass 100% of T (Total).
(3) However, when a user calls fulfillOffer, _computeOfferPricewill calculate P (uint256 offerPrice) to be half of T (total), rather than 0.
(4) When we calculate cfoEarnings (S) using uint256 cfoEarnings = total - offerPrice;, cfoEarnings (S) will only ever be 50% of total (T), rather than up to 100% of T in basis points.

Fix

Either
state that S (offerCut) is only ever expected to be a maximum of 50% of T,
or
change _computeOfferPrice from
return _total * 1e4 / (1e4 + _offerCut);
to
return _total * ((1e4 - _offerCut) / 1e4);

[hwrdtm]

Thanks @TomLeeFounder for your feedback! We have acknowledged it and will be taking it into consideration.

[jordankitty]

Thanks for your participation, @TomLeeFounder! Our team has reviewed your submission, and we are pleased to reward you for your report.

Severity: Note
Points: 10