users can get greifed by accident because of market conditions

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L115

Vulnerability details

details

uint256 previewAmount = this.previewWithdraw(underlyingAmount);

if previewAmount is large number but holder only gives a certian amount to msg.sender then msg.sender cant withdraw there tokens previewAmount is below caluclated

underlyingAmount * IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate   / IRedeemer(redeemer).getExchangeRate(protocol, cToken)

proof of concept

all this depends on market conditions or an attacker can effect market conditions before that makes this possible but in any case this is possible and cause users tyring to withdraw there tokens/allowance to get reverted and it will cause there tokens to get stuck in the contract until market conditions are better. ex:

underlyingAmount=1 
maturiy.rate= 2 
getEchangeRate=1
previeAmount=1*2/1=2
and holder[msg.sender]=1
which will cause the function to revert 
uint256 allowed = allowance[holder][msg.sender];
//1>=2 which would cause else to be exucted 
    if (allowed >= previewAmount) {

                revert Approvals(allowed, previewAmount);

            }
            1-=2 which in solidity 0.8.0^ would cause a revert beacuse underflow 
            allowance[holder][msg.sender] -= previewAmount;

and the function will revert so if maturityRate is down and exchage rate is down for that token then this grefing is very possible.

Recommended Mitigation Steps

only allow the allowacne to get withdrawed ex: alllowed == preivewAmount { previeeAmount=allowed then withdraw that } if allowed is less then previewAmount revert because there is not engough to revert in this case just make allowed=preivemamount so just incase a users can withdraw with there allowance.

[scaraven]

I am not 100% sure what this issue is trying to say but my interpretation is that the issue notices the wrong sign for >= when calculating allowances though the market conditions argument is very shaky. If most of this argument is ignored and the core issue is left then this is essentially a duplicate of #180

[JTraversa]

I dont think thats what the report here is, and instead its a discussion around the precision accuracy when exchangeRates are high/low and the amounts are low.

That said I'm disputing given:

1. Implementation details of the EIP arent in scope
2. The suggestion would actually break the EIP anywho
3. There are precision issues with most Defi protocols, you just kind of deal with them, which is why you need 18 decimals to begin with.

[bghughes]

I agree that this would have been better off as a duplicate of #180 and that this precision claim is esoteric @scaraven @JTraversa.

Moreover, warden I find your argument and math here hard to follow and it uses a ton of arbitrary hard-coded, 1 wei values which seem highly unlikely / impossible to ever occur. Gonna downgrade this to QA as it feels like an edge case and #180 got the real issue here IMO.

[bghughes]

Grouping this with the warden’s QA report, #104