code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

`end()` reverts if bidder blacklisted by collateral token, grieving challenger #974

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Position.sol#L352 https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Position.sol#L269

Vulnerability details

A successful challenge can be ended via MintingHub.end(). This transfers challenge.size collateral back to the challenger, before repaying the challenge and paying the challenger the reward.

In this call, position.notifyChallengeSucceeded is called. This transfers the position's collateral to the highest bidder.

File: Position.sol
351: notifyRepaidInternal(repayment); // we assume the caller takes care of the actual repayment
352:         internalWithdrawCollateral(_bidder, _size); // transfer collateral to the bidder and emit update
File: Position.sol
268: function internalWithdrawCollateral(address target, uint256 amount) internal returns (uint256) {
269:         IERC20(collateral).transfer(target, amount);
270:         uint256 balance = collateralBalance();
271:         if (balance < minimumCollateral){
272:             cooldown = expiration;
273:         }
274:         emitUpdate();
275:         return balance;
276:     }

The issue is that in case of a token with blacklist (USDC, USDT), if the bidder is blacklisted before the end() call, the transfer line 269 will revert

Impact

end() will always revert for this challenge. The challenge cannot be ended and the challenger will never retrieve their size collateral.

Tools Used

Manual Analysis

Mitigation

end() already handles the case where the challenger is blacklisted by such tokens. Add a similar functionality in notifyChallengeSucceeded() for the internalWithdrawCollateral() internal call.

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #675

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #680

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory