hats-finance / VMEX-0xb6861bdeb368a1bf628fc36a36cec62d04fb6a77

LP token lending protocol
MIT License
2 stars 4 forks source link

Mismatch in total supplies can lead to Denial of Service (DoS) in ExternalRewardDistributor.beginStakingReward #9

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @aviggiano Submission hash (on-chain): 0xa039eee6093179e3120b3882e02f6c6962b61db258f2256842a9b200ebafcad3 Severity: low

Description: Description\

In ExternalRewardDistributor.sol:205, a mismatch in the total supply of aToken and its underlying asset can cause a denial of service on the beginStakingReward function. The code assumes that the total supply of the aToken and the underlying should be the same. However, due to potential discrepancies like interest accumulation, aToken’s (an interest-bearing token) total supply can be more or less than the underlying. As a result, the smart contract may attempt to transfer more tokens than available leading to a DoS condition.

Attack Scenario\

If an attacker can manipulate the total supply of either the aToken or its underlying, this could result in a discrepancy causing the attempts to transfer the same amount of tokens to and from the smart contract to fail. This can potentially cause the beginStakingReward function to cease operation.

    uint256 amount = IERC20(aToken).totalSupply();
    if(amount!=0){
      IERC20(underlying).safeTransferFrom(aToken, address(this), amount);
      stake(aToken, amount);
    }

Attachments

  1. Revised Code
diff --git a/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol b/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol
index 166a84eb..040e9396 100644
--- a/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol
+++ b/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol
@@ -202,7 +202,7 @@ contract ExternalRewardDistributor is IExternalRewardsDistributor, Initializable
     }

     //transfer all aToken's underlying to this contract and stake it
-    uint256 amount = IERC20(aToken).totalSupply();
+    uint256 amount = IERC20(underlying).totalSupply();
     if(amount!=0){
       IERC20(underlying).safeTransferFrom(aToken, address(this), amount);
       stake(aToken, amount);

Transfer the totalSupply from the underlying token.

ksyao2002 commented 1 year ago

Hi, thanks for the report. This issue has been commonly reported in the past audit. Since the tokens with external rewards are not borrowable, this would not be a concern. See

https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/issues/57