diff --git a/snapshots/NativeTokenGateway.Operations.json b/snapshots/NativeTokenGateway.Operations.json index 45aeca137..8a06a9d1f 100644 --- a/snapshots/NativeTokenGateway.Operations.json +++ b/snapshots/NativeTokenGateway.Operations.json @@ -1,8 +1,8 @@ { - "borrowNative": "229316", - "repayNative": "168024", - "supplyAsCollateralNative": "160373", - "supplyNative": "136476", - "withdrawNative: full": "125620", - "withdrawNative: partial": "136825" + "borrowNative": "229792", + "repayNative": "168492", + "supplyAsCollateralNative": "161316", + "supplyNative": "136858", + "withdrawNative: full": "125996", + "withdrawNative: partial": "137294" } \ No newline at end of file diff --git a/snapshots/SignatureGateway.Operations.json b/snapshots/SignatureGateway.Operations.json index 96eb0ef3e..5ccaf7d6d 100644 --- a/snapshots/SignatureGateway.Operations.json +++ b/snapshots/SignatureGateway.Operations.json @@ -1,10 +1,10 @@ { - "borrowWithSig": "215605", - "repayWithSig": "188872", + "borrowWithSig": "216081", + "repayWithSig": "189340", "setSelfAsUserPositionManagerWithSig": "75402", - "setUsingAsCollateralWithSig": "85053", - "supplyWithSig": "153205", - "updateUserDynamicConfigWithSig": "62769", - "updateUserRiskPremiumWithSig": "61579", - "withdrawWithSig": "131696" + "setUsingAsCollateralWithSig": "85519", + "supplyWithSig": "153587", + "updateUserDynamicConfigWithSig": "63235", + "updateUserRiskPremiumWithSig": "62045", + "withdrawWithSig": "132071" } \ No newline at end of file diff --git a/snapshots/Spoke.Getters.json b/snapshots/Spoke.Getters.json index 000034236..25b9526e9 100644 --- a/snapshots/Spoke.Getters.json +++ b/snapshots/Spoke.Getters.json @@ -2,6 +2,6 @@ "getUserAccountData: supplies: 0, borrows: 0": "11937", "getUserAccountData: supplies: 1, borrows: 0": "48600", "getUserAccountData: supplies: 2, borrows: 0": "80378", - "getUserAccountData: supplies: 2, borrows: 1": "100166", - "getUserAccountData: supplies: 2, borrows: 2": "118596" + "getUserAccountData: supplies: 2, borrows: 1": "100165", + "getUserAccountData: supplies: 2, borrows: 2": "118594" } \ No newline at end of file diff --git a/snapshots/Spoke.Operations.ZeroRiskPremium.json b/snapshots/Spoke.Operations.ZeroRiskPremium.json index ed2faa23d..89861ae66 100644 --- a/snapshots/Spoke.Operations.ZeroRiskPremium.json +++ b/snapshots/Spoke.Operations.ZeroRiskPremium.json @@ -1,33 +1,33 @@ { - "borrow: first": "191325", - "borrow: second action, same reserve": "171297", - "liquidationCall (receiveShares): full": "300103", - "liquidationCall (receiveShares): partial": "299821", - "liquidationCall: full": "310468", - "liquidationCall: partial": "310186", - "permitReserve + repay (multicall)": "166029", - "permitReserve + supply (multicall)": "146862", - "permitReserve + supply + enable collateral (multicall)": "160573", - "repay: full": "126094", - "repay: partial": "130983", + "borrow: first": "191801", + "borrow: second action, same reserve": "171773", + "liquidationCall (receiveShares): full": "300567", + "liquidationCall (receiveShares): partial": "300285", + "liquidationCall: full": "310932", + "liquidationCall: partial": "310650", + "permitReserve + repay (multicall)": "166498", + "permitReserve + supply (multicall)": "147339", + "permitReserve + supply + enable collateral (multicall)": "161516", + "repay: full": "126563", + "repay: partial": "131452", "setUserPositionManagerWithSig: disable": "44846", "setUserPositionManagerWithSig: enable": "68875", - "supply + enable collateral (multicall)": "140624", - "supply: 0 borrows, collateral disabled": "123679", - "supply: 0 borrows, collateral enabled": "106601", - "supply: second action, same reserve": "106579", - "updateUserDynamicConfig: 1 collateral": "73694", - "updateUserDynamicConfig: 2 collaterals": "88551", - "updateUserRiskPremium: 1 borrow": "94804", - "updateUserRiskPremium: 2 borrows": "104619", - "usingAsCollateral: 0 borrows, enable": "58915", - "usingAsCollateral: 1 borrow, disable": "105072", - "usingAsCollateral: 1 borrow, enable": "41803", - "usingAsCollateral: 2 borrows, disable": "126055", - "usingAsCollateral: 2 borrows, enable": "41815", - "withdraw: 0 borrows, full": "128910", - "withdraw: 0 borrows, partial": "133473", - "withdraw: 1 borrow, partial": "161036", - "withdraw: 2 borrows, partial": "174214", - "withdraw: non collateral": "106544" + "supply + enable collateral (multicall)": "141567", + "supply: 0 borrows, collateral disabled": "124156", + "supply: 0 borrows, collateral enabled": "107078", + "supply: second action, same reserve": "107056", + "updateUserDynamicConfig: 1 collateral": "74160", + "updateUserDynamicConfig: 2 collaterals": "89017", + "updateUserRiskPremium: 1 borrow": "95269", + "updateUserRiskPremium: 2 borrows": "105083", + "usingAsCollateral: 0 borrows, enable": "59381", + "usingAsCollateral: 1 borrow, disable": "105537", + "usingAsCollateral: 1 borrow, enable": "42269", + "usingAsCollateral: 2 borrows, disable": "126519", + "usingAsCollateral: 2 borrows, enable": "42281", + "withdraw: 0 borrows, full": "129379", + "withdraw: 0 borrows, partial": "133942", + "withdraw: 1 borrow, partial": "161504", + "withdraw: 2 borrows, partial": "174681", + "withdraw: non collateral": "107013" } \ No newline at end of file diff --git a/snapshots/Spoke.Operations.json b/snapshots/Spoke.Operations.json index deed9c95d..18a9783f5 100644 --- a/snapshots/Spoke.Operations.json +++ b/snapshots/Spoke.Operations.json @@ -1,33 +1,33 @@ { - "borrow: first": "261721", - "borrow: second action, same reserve": "204693", - "liquidationCall (receiveShares): full": "333666", - "liquidationCall (receiveShares): partial": "333384", - "liquidationCall: full": "344031", - "liquidationCall: partial": "343749", - "permitReserve + repay (multicall)": "163273", - "permitReserve + supply (multicall)": "146862", - "permitReserve + supply + enable collateral (multicall)": "160573", - "repay: full": "120256", - "repay: partial": "139545", + "borrow: first": "262197", + "borrow: second action, same reserve": "205169", + "liquidationCall (receiveShares): full": "334130", + "liquidationCall (receiveShares): partial": "333848", + "liquidationCall: full": "344495", + "liquidationCall: partial": "344213", + "permitReserve + repay (multicall)": "163648", + "permitReserve + supply (multicall)": "147339", + "permitReserve + supply + enable collateral (multicall)": "161516", + "repay: full": "120725", + "repay: partial": "140014", "setUserPositionManagerWithSig: disable": "44846", "setUserPositionManagerWithSig: enable": "68875", - "supply + enable collateral (multicall)": "140624", - "supply: 0 borrows, collateral disabled": "123679", - "supply: 0 borrows, collateral enabled": "106601", - "supply: second action, same reserve": "106579", - "updateUserDynamicConfig: 1 collateral": "73694", - "updateUserDynamicConfig: 2 collaterals": "88551", - "updateUserRiskPremium: 1 borrow": "151080", - "updateUserRiskPremium: 2 borrows": "204276", - "usingAsCollateral: 0 borrows, enable": "58915", - "usingAsCollateral: 1 borrow, disable": "161348", - "usingAsCollateral: 1 borrow, enable": "41803", - "usingAsCollateral: 2 borrows, disable": "233712", - "usingAsCollateral: 2 borrows, enable": "41815", - "withdraw: 0 borrows, full": "128910", - "withdraw: 0 borrows, partial": "133473", - "withdraw: 1 borrow, partial": "214810", - "withdraw: 2 borrows, partial": "259272", - "withdraw: non collateral": "106544" + "supply + enable collateral (multicall)": "141567", + "supply: 0 borrows, collateral disabled": "124156", + "supply: 0 borrows, collateral enabled": "107078", + "supply: second action, same reserve": "107056", + "updateUserDynamicConfig: 1 collateral": "74160", + "updateUserDynamicConfig: 2 collaterals": "89017", + "updateUserRiskPremium: 1 borrow": "151545", + "updateUserRiskPremium: 2 borrows": "204740", + "usingAsCollateral: 0 borrows, enable": "59381", + "usingAsCollateral: 1 borrow, disable": "161813", + "usingAsCollateral: 1 borrow, enable": "42269", + "usingAsCollateral: 2 borrows, disable": "234176", + "usingAsCollateral: 2 borrows, enable": "42281", + "withdraw: 0 borrows, full": "129379", + "withdraw: 0 borrows, partial": "133942", + "withdraw: 1 borrow, partial": "215278", + "withdraw: 2 borrows, partial": "259739", + "withdraw: non collateral": "107013" } \ No newline at end of file diff --git a/src/spoke/Spoke.sol b/src/spoke/Spoke.sol index d71faaa03..ef6cec44a 100644 --- a/src/spoke/Spoke.sol +++ b/src/spoke/Spoke.sol @@ -6,6 +6,7 @@ import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol'; import {SafeERC20, IERC20} from 'src/dependencies/openzeppelin/SafeERC20.sol'; import {IERC20Permit} from 'src/dependencies/openzeppelin/IERC20Permit.sol'; import {SignatureChecker} from 'src/dependencies/openzeppelin/SignatureChecker.sol'; +import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol'; import {AccessManagedUpgradeable} from 'src/dependencies/openzeppelin-upgradeable/AccessManagedUpgradeable.sol'; import {EIP712} from 'src/dependencies/solady/EIP712.sol'; import {MathUtils} from 'src/libraries/math/MathUtils.sol'; @@ -26,7 +27,14 @@ import {ISpokeBase, ISpoke} from 'src/spoke/interfaces/ISpoke.sol'; /// @author Aave Labs /// @notice Handles risk configuration & borrowing strategy for reserves and user positions. /// @dev Each reserve can be associated with a separate Hub. -abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradeable, EIP712 { +abstract contract Spoke is + ISpoke, + Multicall, + NoncesKeyed, + AccessManagedUpgradeable, + ReentrancyGuardTransient, + EIP712 +{ using SafeCast for *; using SafeERC20 for IERC20; using MathUtils for *; @@ -231,7 +239,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea uint256 reserveId, uint256 amount, address onBehalfOf - ) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) { + ) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) { Reserve storage reserve = _getReserve(reserveId); UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId]; _validateSupply(reserve.flags); @@ -250,7 +258,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea uint256 reserveId, uint256 amount, address onBehalfOf - ) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) { + ) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) { Reserve storage reserve = _getReserve(reserveId); UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId]; _validateWithdraw(reserve.flags); @@ -280,7 +288,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea uint256 reserveId, uint256 amount, address onBehalfOf - ) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) { + ) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) { Reserve storage reserve = _getReserve(reserveId); UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId]; PositionStatus storage positionStatus = _positionStatus[onBehalfOf]; @@ -306,7 +314,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea uint256 reserveId, uint256 amount, address onBehalfOf - ) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) { + ) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) { Reserve storage reserve = _getReserve(reserveId); UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId]; _validateRepay(reserve.flags); @@ -350,7 +358,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea address user, uint256 debtToCover, bool receiveShares - ) external { + ) external nonReentrant { Reserve storage collateralReserve = _getReserve(collateralReserveId); Reserve storage debtReserve = _getReserve(debtReserveId); DynamicReserveConfig storage collateralDynConfig = _dynamicConfig[collateralReserveId][ @@ -404,7 +412,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea uint256 reserveId, bool usingAsCollateral, address onBehalfOf - ) external onlyPositionManager(onBehalfOf) { + ) external nonReentrant onlyPositionManager(onBehalfOf) { _validateSetUsingAsCollateral(_getReserve(reserveId).flags, usingAsCollateral); PositionStatus storage positionStatus = _positionStatus[onBehalfOf]; @@ -424,7 +432,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea } /// @inheritdoc ISpoke - function updateUserRiskPremium(address onBehalfOf) external { + function updateUserRiskPremium(address onBehalfOf) external nonReentrant { if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) { _checkCanCall(msg.sender, msg.data); } @@ -433,7 +441,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea } /// @inheritdoc ISpoke - function updateUserDynamicConfig(address onBehalfOf) external { + function updateUserDynamicConfig(address onBehalfOf) external nonReentrant { if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) { _checkCanCall(msg.sender, msg.data); } diff --git a/tests/Base.t.sol b/tests/Base.t.sol index 424bc3683..ac2bf7ff2 100644 --- a/tests/Base.t.sol +++ b/tests/Base.t.sol @@ -12,6 +12,7 @@ import {console2 as console} from 'forge-std/console2.sol'; // dependencies import {AggregatorV3Interface} from 'src/dependencies/chainlink/AggregatorV3Interface.sol'; import {TransparentUpgradeableProxy, ITransparentUpgradeableProxy} from 'src/dependencies/openzeppelin/TransparentUpgradeableProxy.sol'; +import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol'; import {IERC20Metadata} from 'src/dependencies/openzeppelin/IERC20Metadata.sol'; import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol'; import {IERC20Errors} from 'src/dependencies/openzeppelin/IERC20Errors.sol'; @@ -80,6 +81,7 @@ import {MockSpoke} from 'tests/mocks/MockSpoke.sol'; import {MockERC1271Wallet} from 'tests/mocks/MockERC1271Wallet.sol'; import {MockSpokeInstance} from 'tests/mocks/MockSpokeInstance.sol'; import {MockSkimSpoke} from 'tests/mocks/MockSkimSpoke.sol'; +import {MockReentrantHub} from 'tests/mocks/MockReentrantHub.sol'; abstract contract Base is Test { using stdStorage for StdStorage; diff --git a/tests/mocks/MockReentrantHub.sol b/tests/mocks/MockReentrantHub.sol new file mode 100644 index 000000000..13bb41805 --- /dev/null +++ b/tests/mocks/MockReentrantHub.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: UNLICENSED +// Copyright (c) 2025 Aave Labs +pragma solidity ^0.8.0; + +import {Address} from 'src/dependencies/openzeppelin/Address.sol'; + +contract MockReentrantHub { + using Address for address; + + address public immutable TARGET; + bytes4 public immutable TARGET_SELECTOR; + + constructor(address target, bytes4 targetSelector) { + TARGET = target; + TARGET_SELECTOR = targetSelector; + } + + fallback() external { + TARGET.functionCall(bytes.concat(TARGET_SELECTOR, new bytes(1000))); + } +} diff --git a/tests/unit/Spoke/Liquidations/Spoke.LiquidationCall.Scenarios.t.sol b/tests/unit/Spoke/Liquidations/Spoke.LiquidationCall.Scenarios.t.sol index c3ea80aa3..f8b2241e9 100644 --- a/tests/unit/Spoke/Liquidations/Spoke.LiquidationCall.Scenarios.t.sol +++ b/tests/unit/Spoke/Liquidations/Spoke.LiquidationCall.Scenarios.t.sol @@ -52,10 +52,90 @@ contract SpokeLiquidationCallScenariosTest is SpokeLiquidationCallBaseTest { } } + function test_liquidationCall_revertsWith_ReentrancyGuardReentrantCall() public { + uint256 collateralReserveId = _daiReserveId(spoke); + uint256 debtReserveId = _wethReserveId(spoke); + + // _updateTargetHealthFactor(spoke, 1.001e18); + _increaseCollateralSupply(spoke, collateralReserveId, 100000e18, user); + _makeUserLiquidatable(spoke, user, debtReserveId, 0.999e18); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke), + ISpokeBase.liquidationCall.selector + ); + + // reentrant hub.remove call + vm.mockFunction( + address(_hub(spoke, collateralReserveId)), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.remove.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(liquidator); + spoke.liquidationCall(collateralReserveId, debtReserveId, user, type(uint256).max, false); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke, collateralReserveId)), + address(_hub(spoke, collateralReserveId)), + abi.encodeWithSelector(IHubBase.remove.selector) + ); + + // reentrant hub.restore call + vm.mockFunction( + address(_hub(spoke, debtReserveId)), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.restore.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(liquidator); + spoke.liquidationCall(collateralReserveId, debtReserveId, user, type(uint256).max, false); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke, debtReserveId)), + address(_hub(spoke, debtReserveId)), + abi.encodeWithSelector(IHubBase.restore.selector) + ); + + // reentrant hub.refreshPremium + vm.mockFunction( + address(_hub(spoke, debtReserveId)), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(liquidator); + spoke.liquidationCall(collateralReserveId, debtReserveId, user, type(uint256).max, false); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke, debtReserveId)), + address(_hub(spoke, debtReserveId)), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + + _makeUserLiquidatable(spoke, user, debtReserveId, 0.5e18); + + // reentrant hub.restoreDeficit + vm.mockFunction( + address(_hub(spoke, debtReserveId)), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.reportDeficit.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(liquidator); + spoke.liquidationCall(collateralReserveId, debtReserveId, user, type(uint256).max, false); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke, debtReserveId)), + address(_hub(spoke, debtReserveId)), + abi.encodeWithSelector(IHubBase.reportDeficit.selector) + ); + } + // User is solvent, but health factor decreases after liquidation due to high liquidation bonus. // A new collateral factor is set for WETH, but it does not affect the user since dynamic config // key is not refreshed during liquidations. - function test_scenario1() public { + function test_liquidationCall_scenario1() public { // A high liquidation bonus will be applied _updateMaxLiquidationBonus(spoke, _wethReserveId(spoke), 124_00); @@ -157,7 +237,7 @@ contract SpokeLiquidationCallScenariosTest is SpokeLiquidationCallBaseTest { } // User is solvent, but health factor decreases after liquidation due to high collateral factor. - function test_scenario2() public { + function test_liquidationCall_scenario2() public { _updateMaxLiquidationBonus(spoke, _wethReserveId(spoke), 103_00); _updateCollateralFactor(spoke, _wethReserveId(spoke), 97_00); @@ -254,8 +334,8 @@ contract SpokeLiquidationCallScenariosTest is SpokeLiquidationCallBaseTest { assertApproxEqAbs(userAccountData.riskPremium, 13_89, 1, 'post liquidation: risk premium'); } - // Liquidated collateral is between 0 and 1 wei. It is rounded up to prevent reverting. - function test_scenario3() public { + // Liquidated collateral is between 0 and 1 wei. It is rounded down and hub.remove is skipped to avoid reverting. + function test_liquidationCall_scenario3() public { // Liquidation bonus: 0 _updateMaxLiquidationBonus(spoke, _wethReserveId(spoke), 100_00); @@ -306,7 +386,7 @@ contract SpokeLiquidationCallScenariosTest is SpokeLiquidationCallBaseTest { } /// @dev when receiving shares, liquidator can already have setUsingAsCollateral - function test_scenario_liquidator_usingAsCollateral() public { + function test_liquidationCall_scenario4() public { uint256 collateralReserveId = _wethReserveId(spoke); uint256 debtReserveId = _daiReserveId(spoke); // liquidator can receive shares even if they have already set as collateral diff --git a/tests/unit/Spoke/Spoke.Borrow.t.sol b/tests/unit/Spoke/Spoke.Borrow.t.sol index 21c0f7b13..bba908eba 100644 --- a/tests/unit/Spoke/Spoke.Borrow.t.sol +++ b/tests/unit/Spoke/Spoke.Borrow.t.sol @@ -5,6 +5,55 @@ pragma solidity ^0.8.0; import 'tests/unit/Spoke/SpokeBase.t.sol'; contract SpokeBorrowTest is SpokeBase { + function test_borrow_revertsWith_ReentrancyGuardReentrantCall() public { + uint256 amount = 100e18; + + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: amount * 10, + onBehalfOf: bob + }); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpokeBase.borrow.selector + ); + + // reentrant hub.draw call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.draw.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.borrow(_daiReserveId(spoke1), amount, bob); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(_hub(spoke1, _daiReserveId(spoke1))), + abi.encodeWithSelector(IHubBase.draw.selector) + ); + + // reentrant hub.refreshPremium call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.borrow(_daiReserveId(spoke1), amount, bob); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(_hub(spoke1, _daiReserveId(spoke1))), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + } + function test_borrow() public { BorrowTestData memory state; diff --git a/tests/unit/Spoke/Spoke.Repay.t.sol b/tests/unit/Spoke/Spoke.Repay.t.sol index d8b52b0cf..08b711a1f 100644 --- a/tests/unit/Spoke/Spoke.Repay.t.sol +++ b/tests/unit/Spoke/Spoke.Repay.t.sol @@ -58,6 +58,47 @@ contract SpokeRepayTest is SpokeBase { vm.stopPrank(); } + function test_repay_revertsWith_ReentrancyGuardReentrantCall() public { + uint256 amount = 100e18; + + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: amount * 2, + onBehalfOf: bob + }); + + Utils.borrow({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: amount, + onBehalfOf: bob + }); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpokeBase.repay.selector + ); + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeCall( + IHubBase.restore, + ( + daiAssetId, + amount, + _getExpectedPremiumDeltaForRestore(spoke1, bob, _daiReserveId(spoke1), amount) + ) + ) + ); + + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.repay(_daiReserveId(spoke1), amount, bob); + } + function test_repay() public { uint256 daiSupplyAmount = 100e18; uint256 wethSupplyAmount = 10e18; diff --git a/tests/unit/Spoke/Spoke.SetUsingAsCollateral.t.sol b/tests/unit/Spoke/Spoke.SetUsingAsCollateral.t.sol index f5e440fd7..da3e47863 100644 --- a/tests/unit/Spoke/Spoke.SetUsingAsCollateral.t.sol +++ b/tests/unit/Spoke/Spoke.SetUsingAsCollateral.t.sol @@ -52,6 +52,47 @@ contract SpokeConfigTest is SpokeBase { spoke1.setUsingAsCollateral(daiReserveId, true, alice); } + function test_setUsingAsCollateral_revertsWith_ReentrancyGuardReentrantCall() public { + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _wethReserveId(spoke1), + caller: bob, + amount: 1e18, + onBehalfOf: bob + }); + + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: 100e18, + onBehalfOf: bob + }); + + Utils.borrow({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: 100e18, + onBehalfOf: bob + }); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpoke.setUsingAsCollateral.selector + ); + + // reentrant hub.refreshPremium call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.setUsingAsCollateral(_daiReserveId(spoke1), false, bob); + } + /// no action taken when collateral status is unchanged function test_setUsingAsCollateral_collateralStatusUnchanged() public { uint256 daiReserveId = _daiReserveId(spoke1); diff --git a/tests/unit/Spoke/Spoke.Supply.t.sol b/tests/unit/Spoke/Spoke.Supply.t.sol index 947b8e174..fc6bbb079 100644 --- a/tests/unit/Spoke/Spoke.Supply.t.sol +++ b/tests/unit/Spoke/Spoke.Supply.t.sol @@ -85,6 +85,24 @@ contract SpokeSupplyTest is SpokeBase { spoke1.supply(_daiReserveId(spoke1), amount, bob); } + function test_supply_revertsWith_ReentrancyGuardReentrantCall() public { + uint256 amount = 100e18; + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpokeBase.supply.selector + ); + + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.add.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.supply(_daiReserveId(spoke1), amount, bob); + } + function test_supply() public { uint256 amount = 100e18; TestUserData[2] memory bobData; diff --git a/tests/unit/Spoke/Spoke.UpdateUserDynamicConfig.t.sol b/tests/unit/Spoke/Spoke.UpdateUserDynamicConfig.t.sol new file mode 100644 index 000000000..26852a345 --- /dev/null +++ b/tests/unit/Spoke/Spoke.UpdateUserDynamicConfig.t.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: UNLICENSED +// Copyright (c) 2025 Aave Labs +pragma solidity ^0.8.0; + +import 'tests/unit/Spoke/SpokeBase.t.sol'; + +contract SpokeUpdateUserDynamicConfigTest is SpokeBase { + function test_updateUserDynamicConfig_revertsWith_ReentrancyGuardReentrantCall() public { + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: 1000e18, + onBehalfOf: bob + }); + + Utils.borrow({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: 100e18, + onBehalfOf: bob + }); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpoke.updateUserDynamicConfig.selector + ); + + // reentrant hub.refreshPremium call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.updateUserDynamicConfig(bob); + } +} diff --git a/tests/unit/Spoke/Spoke.UpdateUserRiskPremium.t.sol b/tests/unit/Spoke/Spoke.UpdateUserRiskPremium.t.sol new file mode 100644 index 000000000..376dea7a9 --- /dev/null +++ b/tests/unit/Spoke/Spoke.UpdateUserRiskPremium.t.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: UNLICENSED +// Copyright (c) 2025 Aave Labs +pragma solidity ^0.8.0; + +import 'tests/unit/Spoke/SpokeBase.t.sol'; + +contract SpokeUpdateUserRiskPremiumTest is SpokeBase { + function test_updateUserRiskPremium_revertsWith_ReentrancyGuardReentrantCall() public { + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: 1000e18, + onBehalfOf: bob + }); + + Utils.borrow({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: 100e18, + onBehalfOf: bob + }); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpoke.updateUserRiskPremium.selector + ); + + // reentrant hub.refreshPremium call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.updateUserRiskPremium(bob); + } +} diff --git a/tests/unit/Spoke/Spoke.Withdraw.t.sol b/tests/unit/Spoke/Spoke.Withdraw.t.sol index 7b3793865..491368965 100644 --- a/tests/unit/Spoke/Spoke.Withdraw.t.sol +++ b/tests/unit/Spoke/Spoke.Withdraw.t.sol @@ -35,6 +35,63 @@ contract SpokeWithdrawTest is SpokeBase { uint256 skipTime; } + function test_withdraw_revertsWith_ReentrancyGuardReentrantCall() public { + uint256 amount = 100e18; + + Utils.supplyCollateral({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: amount * 10, + onBehalfOf: bob + }); + + MockReentrantHub reentrantHub = new MockReentrantHub( + address(spoke1), + ISpokeBase.withdraw.selector + ); + + // reentrant hub.remove call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.remove.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.withdraw(_daiReserveId(spoke1), amount, bob); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(_hub(spoke1, _daiReserveId(spoke1))), + abi.encodeWithSelector(IHubBase.remove.selector) + ); + + Utils.borrow({ + spoke: spoke1, + reserveId: _daiReserveId(spoke1), + caller: bob, + amount: amount, + onBehalfOf: bob + }); + + // reentrant hub.refreshPremium call + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(reentrantHub), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + vm.expectRevert(ReentrancyGuardTransient.ReentrancyGuardReentrantCall.selector); + vm.prank(bob); + spoke1.withdraw(_daiReserveId(spoke1), amount, bob); + // clear mockFunction + vm.mockFunction( + address(_hub(spoke1, _daiReserveId(spoke1))), + address(_hub(spoke1, _daiReserveId(spoke1))), + abi.encodeWithSelector(IHubBase.refreshPremium.selector) + ); + } + function test_withdraw_same_block() public { uint256 amount = 100e18;