There are several inconsistencies with tax calculations and refunds that leads to users getting an incorrect amount of funds back.
Vulnerability Detail
When a user is first charged tax, the amount of tax that will be levied is automatically: userTaxAmount = (amount * userTxRate) / POINT_BASE; because taxFreeAllcOfUser is hardcoded to 0 in _processPrivate in TokenSale.sol. However, inside the claim function in TokenSale.sol, the taxFreeAllc is computed differently and consequently the refund amount looks different:
This computation is just incorrect because, even if you assume the intent is to tax the user the full amount and then refund some amount of tax, in the case that taxFreeAllc < s.share, the user is not refunded the full amount of tax they should be refunded. They should be refunded ((left + taxFreeAllc) * tax / POINT_BASE) amount of tax instead (otherwise users will purposely get themselves into a state where s.share is just under taxFreeAllc).
Another thing that is incorrect about this is earlier in the claim function, we have a require(left > 0, "TokenSale: Nothing to claim");. However, clearly, left > 0 is not required to receive a tax refund based on the current design, so users who have left = 0 are just missing out on this refund for no reason.
s1ce
high
Inconsistency with tax calculation and refunds
Summary
There are several inconsistencies with tax calculations and refunds that leads to users getting an incorrect amount of funds back.
Vulnerability Detail
When a user is first charged tax, the amount of tax that will be levied is automatically:
userTaxAmount = (amount * userTxRate) / POINT_BASE;
becausetaxFreeAllcOfUser
is hardcoded to0
in_processPrivate
inTokenSale.sol
. However, inside theclaim
function inTokenSale.sol
, thetaxFreeAllc
is computed differently and consequently the refund amount looks different:This computation is just incorrect because, even if you assume the intent is to tax the user the full amount and then refund some amount of tax, in the case that
taxFreeAllc < s.share
, the user is not refunded the full amount of tax they should be refunded. They should be refunded((left + taxFreeAllc) * tax / POINT_BASE)
amount of tax instead (otherwise users will purposely get themselves into a state wheres.share
is just undertaxFreeAllc
).Another thing that is incorrect about this is earlier in the
claim
function, we have arequire(left > 0, "TokenSale: Nothing to claim");
. However, clearly,left > 0
is not required to receive a tax refund based on the current design, so users who haveleft = 0
are just missing out on this refund for no reason.Impact
Users receive incorrect amount of tax refund back
Code Snippet
https://github.com/sherlock-audit/2024-03-zap-protocol/blob/main/zap-contracts-labs/contracts/TokenSale.sol#L364
Tool used
Manual Review
Recommendation
See above; change how
refundTaxAmount
is calculatedDuplicate of #57