code-423n4 / 2023-06-lybra-findings

8 stars 7 forks source link

Providers can lose part of reward due to lack of access control and frontrunning #703

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L173 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L140

Vulnerability details

Impact

Providers can lose part of reward due to frontrunning

Proof of Concept

The liquidation() function after execution, transfers the reduceAsset reward to provider but when called by the keeper, the keeper reward is deducted from the providers reward and sent to the keeper.

if (provider == msg.sender) {
            collateralAsset.transfer(msg.sender, reducedAsset);
        } else {
            reward2keeper = (reducedAsset * configurator.vaultKeeperRatio(address(this))) / 110;
            collateralAsset.transfer(provider, reducedAsset - reward2keeper); // @audit amount get deducted from providers reward and sent to msg.sender
            collateralAsset.transfer(msg.sender, reward2keeper); 
        }

However due to lack of verification or access control on msg.sender, anyone not necessarily the keeper at anytime can perform this and take keeper reward from provider's expected reward after the provider has given allowance to the contract by frontrunning. This attack can be executed on the providers every single time they try liquidating a borrower

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Consider implementing an access control or permission on the msg.sender

Assessed type

Access Control

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Insufficient quality