fix(contract): remove shares exploit fix

pooltogether · Jul 6, 2022 · 64d0013 · 64d0013
1 parent b25ef74
commit 64d0013
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 43 deletions.
diff --git a/contracts/AaveV3YieldSource.sol b/contracts/AaveV3YieldSource.sol
@@ -232,18 +232,15 @@ contract AaveV3YieldSource is ERC20, IYieldSource, Manageable, ReentrancyGuard {
    * @param _to The user whose balance will receive the tokens
    */
   function supplyTokenTo(uint256 _depositAmount, address _to) external override nonReentrant {
-    uint256 _fullShare = _pricePerShare();
-
-    uint256 _shares = _tokenToShares(_depositAmount, _fullShare);
+    uint256 _shares = _tokenToShares(_depositAmount, _pricePerShare());
     _requireSharesGTZero(_shares);
 
-    uint256 _tokenAmount = _sharesToToken(_shares, _fullShare);
-    IERC20(_tokenAddress).safeTransferFrom(msg.sender, address(this), _tokenAmount);
-    _pool().supply(_tokenAddress, _tokenAmount, address(this), REFERRAL_CODE);
+    IERC20(_tokenAddress).safeTransferFrom(msg.sender, address(this), _depositAmount);
+    _pool().supply(_tokenAddress, _depositAmount, address(this), REFERRAL_CODE);
 
     _mint(_to, _shares);
 
-    emit SuppliedTokenTo(msg.sender, _shares, _tokenAmount, _to);
+    emit SuppliedTokenTo(msg.sender, _shares, _depositAmount, _to);
   }
 
   /**
@@ -254,18 +251,14 @@ contract AaveV3YieldSource is ERC20, IYieldSource, Manageable, ReentrancyGuard {
    * @return The actual amount of asset tokens that were redeemed.
    */
   function redeemToken(uint256 _redeemAmount) external override nonReentrant returns (uint256) {
-    uint256 _fullShare = _pricePerShare();
-
-    uint256 _shares = _tokenToShares(_redeemAmount, _fullShare);
+    uint256 _shares = _tokenToShares(_redeemAmount, _pricePerShare());
     _requireSharesGTZero(_shares);
 
-    uint256 _tokenAmount = _sharesToToken(_shares, _fullShare);
-
     _burn(msg.sender, _shares);
 
     IERC20 _assetToken = IERC20(_tokenAddress);
     uint256 _beforeBalance = _assetToken.balanceOf(address(this));
-    _pool().withdraw(_tokenAddress, _tokenAmount, address(this));
+    _pool().withdraw(_tokenAddress, _redeemAmount, address(this));
 
     uint256 _balanceDiff;
 
@@ -275,7 +268,7 @@ contract AaveV3YieldSource is ERC20, IYieldSource, Manageable, ReentrancyGuard {
 
     _assetToken.safeTransfer(msg.sender, _balanceDiff);
 
-    emit RedeemedToken(msg.sender, _shares, _tokenAmount);
+    emit RedeemedToken(msg.sender, _shares, _redeemAmount);
     return _balanceDiff;
   }
 

diff --git a/test/AaveV3YieldSource.test.ts b/test/AaveV3YieldSource.test.ts
@@ -481,35 +481,6 @@ describe('AaveV3YieldSource', () => {
         aaveV3YieldSource.connect(attacker).redeemToken(attackerRedeemAmount),
       ).to.be.revertedWith('AaveV3YS/shares-gt-zero');
     });
-
-    it('should succeed to manipulate share price but fail to redeem more than deposited', async () => {
-      const amount = toWei('100000');
-      const attackAmount = BigNumber.from(1);
-      const aTokenAmount = toWei('10000');
-
-      await supplyTokenTo(attacker, attackAmount);
-
-      // Attacker sends 10000 aTokens directly to the contract to manipulate share price
-      await aToken.mint(attacker.address, aTokenAmount);
-      await aToken.connect(attacker).approve(aaveV3YieldSource.address, aTokenAmount);
-      await aToken.connect(attacker).transfer(aaveV3YieldSource.address, aTokenAmount);
-
-      await supplyTokenTo(wallet2, amount);
-
-      const sharePrice = await aaveV3YieldSource.sharesToToken(BigNumber.from(1));
-
-      // Redeem 1 wei less than the full amount to burn 1 share instead of 2 because of rounding error
-      // The actual amount of shares to be burnt should be 1.99 but since Solidity truncates down, it will be 1
-      const attackerRedeemAmount = sharePrice.mul(2).sub(1);
-
-      const attackerRedeemShare = await aaveV3YieldSource.tokenToShares(attackerRedeemAmount);
-      const redeemAmount = await aaveV3YieldSource.sharesToToken(attackerRedeemShare);
-
-      await aaveV3YieldSource.connect(attacker).redeemToken(attackerRedeemAmount);
-
-      expect(await usdcToken.balanceOf(attacker.address)).to.equal(redeemAmount);
-      expect(await aaveV3YieldSource.balanceOfToken(attacker.address)).to.equal(Zero);
-    });
   });
 
   describe('claimRewards()', () => {