Lack of slippage in `redeem` can result in loss of shares for redeemer

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Equity.sol#L276-L283

Vulnerability details

FPS holders can redeem their shares against zchf using redeem()

File: Equity.sol
276: function redeem(address target, uint256 shares) public returns (uint256) {//@audit no slippage, calculateProceeds() can return 0
277:         require(canRedeem(msg.sender));
278:         uint256 proceeds = calculateProceeds(shares);
279:         _burn(msg.sender, shares);
280:         zchf.transfer(target, proceeds);
281:         emit Trade(msg.sender, -int(shares), proceeds, price());
282:         return proceeds;
283:     }

The issue is that there is no slippage in this function. As the amount of zchf computed in calculateProceeds() is a function of equity, events happening in the same block before the redeem() call can affect the amount of zchf received by the user

Impact

Users can receive less than expected.

But if a big equity loss happens resulting in zchf.equity returning 0, calculateProceeds will return 0, meaning the user will see their shares being burnt without receiving any zchf.

While it can be argued that the user shares were worth nothing due to the equity loss, in the case of the "brave soul" ready to save the protocol - mentioned in the case scenario here, they will have lost the ability and incentive to do so.

Tools Used

Manual Analysis

Mitigation

Add a slippage parameter in redeem()

Users will be able to estimate how much they can expect using calculateProceeds().

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #396

[c4-judge]

hansfriese marked the issue as selected for report

[c4-judge]

hansfriese marked the issue as not selected for report

[c4-judge]

hansfriese marked the issue as duplicate of #396

[c4-judge]

hansfriese marked the issue as satisfactory