diff --git a/audits/11-09-2024_Certora_StataTokenV2.pdf b/audits/11-09-2024_Certora_StataTokenV2.pdf index 1d6e8654..b908d33e 100644 Binary files a/audits/11-09-2024_Certora_StataTokenV2.pdf and b/audits/11-09-2024_Certora_StataTokenV2.pdf differ diff --git a/certora/stata/specs/StataToken/StataToken.spec b/certora/stata/specs/StataToken/StataToken.spec index 479816b3..ee1c3ef5 100644 --- a/certora/stata/specs/StataToken/StataToken.spec +++ b/certora/stata/specs/StataToken/StataToken.spec @@ -102,8 +102,8 @@ import "../methods/methods_base.spec"; f.contract == currentContract && !harnessOnlyMethods(f) && f.selector != sig:initialize(address, string, string).selector) && - f.selector != sig:emergencyEtherTransfer(uint256).selector && - f.selector != sig:emergencyTokenTransfer(address,uint256).selector + f.selector != sig:emergencyEtherTransfer(address,uint256).selector && + f.selector != sig:emergencyTokenTransfer(address,address,uint256).selector } { // Assuming single reward single_RewardToken_setup(); @@ -137,7 +137,7 @@ import "../methods/methods_base.spec"; && !harnessMethodsMinusHarnessClaimMethods(f) && !claimFunctions(f) && f.selector != sig:claimDoubleRewardOnBehalfSame(address, address, address).selector - && f.selector != sig:emergencyEtherTransfer(uint256).selector + && f.selector != sig:emergencyEtherTransfer(address,uint256).selector } { preserved redeem(uint256 shares, address receiver, address owner) with (env e1) { @@ -152,7 +152,7 @@ import "../methods/methods_base.spec"; requireInvariant solvency_total_asset_geq_total_supply(); require balanceOf(owner) <= totalSupply(); } - preserved emergencyTokenTransfer(address asset, uint256 amount) with (env e3) { + preserved emergencyTokenTransfer(address asset, address to, uint256 amount) with (env e3) { require rate() >= RAY(); } } @@ -170,7 +170,7 @@ import "../methods/methods_base.spec"; f.contract == currentContract && !harnessMethodsMinusHarnessClaimMethods(f) && !claimFunctions(f) - && f.selector != sig:emergencyEtherTransfer(uint256).selector + && f.selector != sig:emergencyEtherTransfer(address,uint256).selector && f.selector != sig:claimDoubleRewardOnBehalfSame(address, address, address).selector } { preserved withdraw(uint256 assets, address receiver, address owner) with (env e3) { @@ -198,7 +198,7 @@ import "../methods/methods_base.spec"; preserved redeemATokens(uint256 shares, address receiver, address owner) with (env e2) { require balanceOf(owner) <= totalSupply(); } - preserved emergencyTokenTransfer(address asset, uint256 amount) with (env e1) { + preserved emergencyTokenTransfer(address asset, address to, uint256 amount) with (env e1) { require rate() >= RAY(); } } @@ -216,7 +216,7 @@ import "../methods/methods_base.spec"; => (_RewardsController.getUserAccruedReward(_asset, reward, user) == _RewardsController.getUserAccruedRewards(reward, user))) filtered {f -> f.contract == currentContract && - f.selector != sig:emergencyEtherTransfer(uint256).selector && + f.selector != sig:emergencyEtherTransfer(address,uint256).selector && !harnessOnlyMethods(f) } { @@ -256,8 +256,8 @@ import "../methods/methods_base.spec"; && !collectAndUpdateFunction(f) && !harnessOnlyMethods(f) && f.selector != sig:initialize(address,string,string).selector - && f.selector != sig:emergencyEtherTransfer(uint256).selector - && f.selector != sig:emergencyTokenTransfer(address,uint256).selector + && f.selector != sig:emergencyEtherTransfer(address,uint256).selector + && f.selector != sig:emergencyTokenTransfer(address,address,uint256).selector } { env e; @@ -301,7 +301,7 @@ rule getClaimableRewards_stable(method f) && !claimFunctions(f) && !collectAndUpdateFunction(f) && f.selector != sig:initialize(address,string,string).selector - && f.selector != sig:emergencyEtherTransfer(uint256).selector + && f.selector != sig:emergencyEtherTransfer(address,uint256).selector && !harnessOnlyMethods(f) } { diff --git a/certora/stata/specs/StataToken/aTokenProperties.spec b/certora/stata/specs/StataToken/aTokenProperties.spec index be0f9fad..55d4e705 100644 --- a/certora/stata/specs/StataToken/aTokenProperties.spec +++ b/certora/stata/specs/StataToken/aTokenProperties.spec @@ -193,7 +193,7 @@ import "../methods/methods_base.spec"; !f.isView && f.selector != sig:redeem(uint256,address,address).selector && f.selector != sig:redeemATokens(uint256,address,address).selector && - f.selector != sig:emergencyEtherTransfer(uint256).selector && + f.selector != sig:emergencyEtherTransfer(address,uint256).selector && !harnessOnlyMethods(f)} { preserved with (env e){ diff --git a/certora/stata/specs/erc4626/erc4626Extended.spec b/certora/stata/specs/erc4626/erc4626Extended.spec index cbf95cb6..c84370b9 100644 --- a/certora/stata/specs/erc4626/erc4626Extended.spec +++ b/certora/stata/specs/erc4626/erc4626Extended.spec @@ -235,7 +235,7 @@ import "../methods/methods_base.spec"; f.contract == currentContract && !f.isView && !harnessOnlyMethods(f) && - f.selector != sig:emergencyEtherTransfer(uint256).selector && + f.selector != sig:emergencyEtherTransfer(address,uint256).selector && f.selector != sig:deposit(uint256,address).selector && f.selector != sig:depositWithPermit(uint256,address,uint256,IERC4626StataToken.SignatureParams,bool).selector && f.selector != sig:withdraw(uint256,address,address).selector && diff --git a/src/contracts/extensions/paraswap-adapters/BaseParaSwapBuyAdapter.sol b/src/contracts/extensions/paraswap-adapters/BaseParaSwapBuyAdapter.sol index 5d9db556..abff3e87 100644 --- a/src/contracts/extensions/paraswap-adapters/BaseParaSwapBuyAdapter.sol +++ b/src/contracts/extensions/paraswap-adapters/BaseParaSwapBuyAdapter.sol @@ -39,6 +39,7 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { * @param maxAmountToSwap Max amount to be swapped * @param amountToReceive Amount to be received from the swap * @return amountSold The amount sold during the swap + * @return amountBought The amount bought during the swap */ function _buyOnParaSwap( uint256 toAmountOffset, @@ -47,7 +48,7 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { IERC20Detailed assetToSwapTo, uint256 maxAmountToSwap, uint256 amountToReceive - ) internal returns (uint256 amountSold) { + ) internal returns (uint256 amountSold, uint256 amountBought) { (bytes memory buyCalldata, IParaSwapAugustus augustus) = abi.decode( paraswapData, (bytes, IParaSwapAugustus) @@ -75,7 +76,6 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { uint256 balanceBeforeAssetTo = assetToSwapTo.balanceOf(address(this)); address tokenTransferProxy = augustus.getTokenTransferProxy(); - assetToSwapFrom.safeApprove(tokenTransferProxy, 0); assetToSwapFrom.safeApprove(tokenTransferProxy, maxAmountToSwap); if (toAmountOffset != 0) { @@ -101,12 +101,15 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { } } + // Reset allowance + assetToSwapFrom.safeApprove(tokenTransferProxy, 0); + uint256 balanceAfterAssetFrom = assetToSwapFrom.balanceOf(address(this)); amountSold = balanceBeforeAssetFrom - balanceAfterAssetFrom; require(amountSold <= maxAmountToSwap, 'WRONG_BALANCE_AFTER_SWAP'); - uint256 amountReceived = assetToSwapTo.balanceOf(address(this)).sub(balanceBeforeAssetTo); - require(amountReceived >= amountToReceive, 'INSUFFICIENT_AMOUNT_RECEIVED'); + amountBought = assetToSwapTo.balanceOf(address(this)).sub(balanceBeforeAssetTo); + require(amountBought >= amountToReceive, 'INSUFFICIENT_AMOUNT_RECEIVED'); - emit Bought(address(assetToSwapFrom), address(assetToSwapTo), amountSold, amountReceived); + emit Bought(address(assetToSwapFrom), address(assetToSwapTo), amountSold, amountBought); } } diff --git a/src/contracts/extensions/paraswap-adapters/BaseParaSwapSellAdapter.sol b/src/contracts/extensions/paraswap-adapters/BaseParaSwapSellAdapter.sol index 5292a3b8..2b5a306f 100644 --- a/src/contracts/extensions/paraswap-adapters/BaseParaSwapSellAdapter.sol +++ b/src/contracts/extensions/paraswap-adapters/BaseParaSwapSellAdapter.sol @@ -98,6 +98,7 @@ abstract contract BaseParaSwapSellAdapter is BaseParaSwapAdapter { revert(0, returndatasize()) } } + require( assetToSwapFrom.balanceOf(address(this)) == balanceBeforeAssetFrom - amountToSwap, 'WRONG_BALANCE_AFTER_SWAP' diff --git a/src/contracts/extensions/paraswap-adapters/ParaSwapRepayAdapter.sol b/src/contracts/extensions/paraswap-adapters/ParaSwapRepayAdapter.sol index 34cf416c..659aae7c 100644 --- a/src/contracts/extensions/paraswap-adapters/ParaSwapRepayAdapter.sol +++ b/src/contracts/extensions/paraswap-adapters/ParaSwapRepayAdapter.sol @@ -114,7 +114,7 @@ contract ParaSwapRepayAdapter is BaseParaSwapBuyAdapter, ReentrancyGuard { // Pull aTokens from user _pullATokenAndWithdraw(address(collateralAsset), msg.sender, collateralAmount, permitSignature); //buy debt asset using collateral asset - uint256 amountSold = _buyOnParaSwap( + (uint256 amountSold, uint256 amountBought) = _buyOnParaSwap( buyAllBalanceOffset, paraswapData, collateralAsset, @@ -127,15 +127,23 @@ contract ParaSwapRepayAdapter is BaseParaSwapBuyAdapter, ReentrancyGuard { //deposit collateral back in the pool, if left after the swap(buy) if (collateralBalanceLeft > 0) { - IERC20(collateralAsset).safeApprove(address(POOL), 0); IERC20(collateralAsset).safeApprove(address(POOL), collateralBalanceLeft); POOL.deposit(address(collateralAsset), collateralBalanceLeft, msg.sender, 0); + IERC20(collateralAsset).safeApprove(address(POOL), 0); } // Repay debt. Approves 0 first to comply with tokens that implement the anti frontrunning approval fix - IERC20(debtAsset).safeApprove(address(POOL), 0); IERC20(debtAsset).safeApprove(address(POOL), debtRepayAmount); POOL.repay(address(debtAsset), debtRepayAmount, debtRateMode, msg.sender); + IERC20(debtAsset).safeApprove(address(POOL), 0); + + { + //transfer excess of debtAsset back to the user, if any + uint256 debtAssetExcess = amountBought - debtRepayAmount; + if (debtAssetExcess > 0) { + IERC20(debtAsset).safeTransfer(msg.sender, debtAssetExcess); + } + } } /** @@ -170,7 +178,7 @@ contract ParaSwapRepayAdapter is BaseParaSwapBuyAdapter, ReentrancyGuard { initiator ); - uint256 amountSold = _buyOnParaSwap( + (uint256 amountSold, uint256 amountBought) = _buyOnParaSwap( buyAllBalanceOffset, paraswapData, collateralAsset, @@ -180,9 +188,9 @@ contract ParaSwapRepayAdapter is BaseParaSwapBuyAdapter, ReentrancyGuard { ); // Repay debt. Approves for 0 first to comply with tokens that implement the anti frontrunning approval fix. - IERC20(debtAsset).safeApprove(address(POOL), 0); IERC20(debtAsset).safeApprove(address(POOL), debtRepayAmount); POOL.repay(address(debtAsset), debtRepayAmount, rateMode, initiator); + IERC20(debtAsset).safeApprove(address(POOL), 0); uint256 neededForFlashLoanRepay = amountSold.add(premium); @@ -194,6 +202,14 @@ contract ParaSwapRepayAdapter is BaseParaSwapBuyAdapter, ReentrancyGuard { permitSignature ); + { + //transfer excess of debtAsset back to the user, if any + uint256 debtAssetExcess = amountBought - debtRepayAmount; + if (debtAssetExcess > 0) { + IERC20(debtAsset).safeTransfer(initiator, debtAssetExcess); + } + } + // Repay flashloan. Approves for 0 first to comply with tokens that implement the anti frontrunning approval fix. IERC20(collateralAsset).safeApprove(address(POOL), 0); IERC20(collateralAsset).safeApprove(address(POOL), collateralAmount.add(premium)); diff --git a/src/contracts/extensions/static-a-token/StataTokenV2.sol b/src/contracts/extensions/static-a-token/StataTokenV2.sol index 561c34c5..f2af30b7 100644 --- a/src/contracts/extensions/static-a-token/StataTokenV2.sol +++ b/src/contracts/extensions/static-a-token/StataTokenV2.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.0; import {ERC20Upgradeable, ERC20PermitUpgradeable} from 'openzeppelin-contracts-upgradeable/contracts/token/ERC20/extensions/ERC20PermitUpgradeable.sol'; import {IERC20Metadata} from '@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol'; import {PausableUpgradeable} from 'openzeppelin-contracts-upgradeable/contracts/utils/PausableUpgradeable.sol'; -import {IPermissionlessRescuable, PermissionlessRescuable} from 'solidity-utils/contracts/utils/PermissionlessRescuable.sol'; +import {IRescuable, Rescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; import {IRescuableBase, RescuableBase} from 'solidity-utils/contracts/utils/RescuableBase.sol'; import {IERC20Permit} from '@openzeppelin/contracts/token/ERC20/extensions/IERC20Permit.sol'; @@ -24,7 +24,7 @@ contract StataTokenV2 is ERC20AaveLMUpgradeable, ERC4626StataTokenUpgradeable, PausableUpgradeable, - PermissionlessRescuable, + Rescuable, IStataTokenV2 { using Math for uint256; @@ -59,9 +59,9 @@ contract StataTokenV2 is else _unpause(); } - /// @inheritdoc IPermissionlessRescuable - function whoShouldReceiveFunds() public view override returns (address) { - return IAToken(address(aToken())).RESERVE_TREASURY_ADDRESS(); + /// @inheritdoc IRescuable + function whoCanRescue() public view override returns (address) { + return POOL_ADDRESSES_PROVIDER.getACLAdmin(); } /// @inheritdoc IRescuableBase diff --git a/tests/extensions/static-a-token/StataTokenV2Rescuable.sol b/tests/extensions/static-a-token/StataTokenV2Rescuable.sol index 9a249416..8034fd52 100644 --- a/tests/extensions/static-a-token/StataTokenV2Rescuable.sol +++ b/tests/extensions/static-a-token/StataTokenV2Rescuable.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.10; +import {IRescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; import {IAToken} from '../../../src/contracts/extensions/static-a-token/StataTokenV2.sol'; +import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/extensions/IERC20Metadata.sol'; import {BaseTest} from './TestBase.sol'; contract StataTokenV2RescuableTest is BaseTest { @@ -12,14 +14,28 @@ contract StataTokenV2RescuableTest is BaseTest { uint256 amount ); + function test_rescuable_shouldRevertForInvalidCaller() external { + deal(tokenList.usdx, address(stataTokenV2), 1 ether); + vm.expectRevert('ONLY_RESCUE_GUARDIAN'); + IRescuable(address(stataTokenV2)).emergencyTokenTransfer( + tokenList.usdx, + address(this), + 1 ether + ); + } + function test_rescuable_shouldTransferAssetsToCollector() external { deal(tokenList.usdx, address(stataTokenV2), 1 ether); - stataTokenV2.emergencyTokenTransfer(tokenList.usdx, 1 ether); + vm.startPrank(poolAdmin); + stataTokenV2.emergencyTokenTransfer(tokenList.usdx, address(this), 1 ether); + assertEq(IERC20(tokenList.usdx).balanceOf(address(this)), 1 ether); } function test_rescuable_shouldWorkForAToken() external { _fundAToken(1 ether, address(stataTokenV2)); - stataTokenV2.emergencyTokenTransfer(aToken, 1 ether); + vm.startPrank(poolAdmin); + stataTokenV2.emergencyTokenTransfer(aToken, address(this), 1 ether); + assertApproxEqAbs(IERC20(aToken).balanceOf(address(this)), 1 ether, 1); } function test_rescuable_shouldNotCauseInsolvency(uint256 donation, uint256 stake) external { @@ -31,7 +47,8 @@ contract StataTokenV2RescuableTest is BaseTest { address treasury = IAToken(aToken).RESERVE_TREASURY_ADDRESS(); vm.expectEmit(true, true, true, true); - emit ERC20Rescued(address(this), aToken, treasury, donation); - stataTokenV2.emergencyTokenTransfer(aToken, donation + stake); + emit ERC20Rescued(poolAdmin, aToken, address(this), donation); + vm.startPrank(poolAdmin); + stataTokenV2.emergencyTokenTransfer(aToken, address(this), donation + stake); } }