code-423n4 / 2021-12-vader-findings

0 stars 0 forks source link

mint() function can be frontfun after native/foreign amounts are sent to the pool #59

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

jayjonah8

Vulnerability details

Impact

In BasePool.sol, the mint() functions balanceNative and balanceForeign values are both based on the balanceOf(address(this)). As the comments above the mint function mentions, "Amounts of native and foreign must be sent to the pool prior to calling mint such that balance of pool for both assets must be greater than their corresponding reserves." This means that a user must first transfer the native and foreign amounts into the BasePool.sol contract before they can call mint().

An attacker can simply monitor the blockchain waiting for a user to do the first step of transferring in the native and foreign assets to the contract and then call mint() essentially stealing the users funds right from under them since they could call the mint() function before the user does frontrunning them in that sense. This is possible since the mint() function uses balanceOf(address(this)) for both assets.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex/pool/BasePool.sol#L148

Tools Used

Manual code review

Recommended Mitigation Steps

The problem is that the user has to send in the funds before performing the second transaction of calling the mint() function. Consider using safeTransferFrom to grab the tokens from the user and then performing the proper math checks on the amounts received rather than relying on balanceOf(address(this)).

jack-the-pug commented 2 years ago

It's quite clear that this function is supposed to be called by a smart contact rather than a EOA.