search

code-423n4 / 2021-10-defiprotocol-findings

0 stars 0 forks source link

If newRatio > ibRatio after a settlement the protocol could lose its funds. #79

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

tensors

Vulnerability details

Suppose that after a certain settleAuction call we have that newRatio > ibRatio. I don't see any reason why this couldn't be possible, going through the math and solving for this condition we can see that:

if b > ibRatio then newRatio > ibRatio (assuming factory.auctionMultiplier = 2). Where b is defined here: https://github.com/code-423n4/2021-10-defiprotocol/blob/7ca848f2779e2e64ed0b4756c02f0137ecd73e50/contracts/contracts/Auction.sol#L95-L97

A user can wait until this settleAuction will be called, mint() before the auction with a large amount, wait until the settleAuction is called and updates the ibRatio: https://github.com/code-423n4/2021-10-defiprotocol/blob/7ca848f2779e2e64ed0b4756c02f0137ecd73e50/contracts/contracts/Auction.sol#L108

After the ratio is updated the user can burn() the tokens he obtained and receive more tokens back after the ibRatio is increased.

An additional concern is the dependency on factory variables in the calculation: .auctionMultiplier and .auctionDecrement These can be changed by the factory deployer, which means that the changes could potentially change newRatio to be greater than ibRatio, allowing the attack to occur.

frank-beard commented 2 years ago

minting/burning is disabled during an auction, so the ibratio cannot be updated in this way

GalloDaSballo commented 2 years ago

As per the sponsor statement, the finding is invalid because you can't change ibRatio via mint / burn as those are locked during the auction.

For the sponsor: Notice that this means that an auction could be created with the goal of causing a DOS against depositor (as they now are dependent on the auctionBonder to settle the auction)