search

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

1 stars 0 forks source link

Transferring only required differences will avoid excessive fund transfer #159

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0xRajeev

Vulnerability details

Impact

Checking tokensNeeded in settleAuction() and transferring only those amounts from msg.sender will prevent them from sending more than required leading to their loss of funds and dust accumulation in basket.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Auction.sol#L81-L99

Tools Used

Manual Analysis

Recommended Mitigation Steps

Check tokensNeeded in settleAuction() and transfer only those amounts from msg.sender instead of inputWeights.

frank-beard commented 2 years ago

not an exploit

GalloDaSballo commented 2 years ago

I personally believe that using tokensNeeded as a core of the logic would make the code more readable and easier to use.

That said, the check against tokensNeeded is there basically ensure that the max amount withdrawn is fair While I would re-write the logic to be simpler, I think this to be safe.

Given the lack of a specific poc, I'm setting this to invalid