WithdrawProxy will break if there is a fee-on-transfer underlying

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/PublicVault.sol#L384-L387

Vulnerability details

PublicVault.transferWithdrawReserve() will increase WithdrawProxy.withdrawReserveReceived by withdrawBalance. The issue is that if the token has a fee-on-transfer, withdrawBalance will be greater than the amount received by withdrawProxy

384: ERC20(asset()).safeTransfer(currentWithdrawProxy, withdrawBalance);
385:       WithdrawProxy(currentWithdrawProxy).increaseWithdrawReserveReceived(
386:         withdrawBalance
387:       );

235: function increaseWithdrawReserveReceived(uint256 amount) external onlyVault {
236:     WPStorage storage s = _loadSlot();
237:     s.withdrawReserveReceived += amount;
238:   }

This can make WithdrawProxy.claim() revert because of underflow:

255: uint256 balance = ERC20(asset()).balanceOf(address(this)) -
256:       s.withdrawReserveReceived; // will never underflow because withdrawReserveReceived is always increased by the transfer amount from the PublicVault 
//@audit - except for fee-on-transfer tokens

meaning processEpoch() reverting, which eventually means commitToLien() reverts, ie users cannot borrow.

USDT is an example token that can have fees activated.

Impact

Medium

Tools Used

Manual Analysis

Mitigation

Consider checking balances to ensure withdrawReserveReceived never exceeds the token balance.

+    balanceBefore = asset().balanceOf(currentWithdrawProxy);
384: ERC20(asset()).safeTransfer(currentWithdrawProxy, withdrawBalance);
+    balanceAfter = asset().balanceOf(currentWithdrawProxy);
385:       WithdrawProxy(currentWithdrawProxy).increaseWithdrawReserveReceived(
-386:         withdrawBalance
+386:         balanceAfter - balanceBefore
387:       );

[c4-judge]

Picodes marked the issue as duplicate of #51

[c4-judge]

Picodes marked the issue as satisfactory