Skip to content

Commit 875b77c

Browse files
avniculaeavniculae
committed
feat: add reentrancy locks in spoke
1 parent 6959e32 commit 875b77c

16 files changed

+484
-87
lines changed

‎snapshots/NativeTokenGateway.Operations.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
2-
"borrowNative": "229316",
3-
"repayNative": "168024",
4-
"supplyAsCollateralNative": "160373",
5-
"supplyNative": "136476",
6-
"withdrawNative: full": "125620",
7-
"withdrawNative: partial": "136825"
2+
"borrowNative": "229792",
3+
"repayNative": "168492",
4+
"supplyAsCollateralNative": "161316",
5+
"supplyNative": "136858",
6+
"withdrawNative: full": "125996",
7+
"withdrawNative: partial": "137294"
88
}

‎snapshots/SignatureGateway.Operations.json‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"borrowWithSig": "215605",
3-
"repayWithSig": "188872",
2+
"borrowWithSig": "216081",
3+
"repayWithSig": "189340",
44
"setSelfAsUserPositionManagerWithSig": "75402",
5-
"setUsingAsCollateralWithSig": "85053",
6-
"supplyWithSig": "153205",
7-
"updateUserDynamicConfigWithSig": "62769",
8-
"updateUserRiskPremiumWithSig": "61579",
9-
"withdrawWithSig": "131696"
5+
"setUsingAsCollateralWithSig": "85519",
6+
"supplyWithSig": "153587",
7+
"updateUserDynamicConfigWithSig": "63235",
8+
"updateUserRiskPremiumWithSig": "62045",
9+
"withdrawWithSig": "132071"
1010
}

‎snapshots/Spoke.Getters.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
"getUserAccountData: supplies: 0, borrows: 0": "11937",
33
"getUserAccountData: supplies: 1, borrows: 0": "48600",
44
"getUserAccountData: supplies: 2, borrows: 0": "80378",
5-
"getUserAccountData: supplies: 2, borrows: 1": "100166",
6-
"getUserAccountData: supplies: 2, borrows: 2": "118596"
5+
"getUserAccountData: supplies: 2, borrows: 1": "100165",
6+
"getUserAccountData: supplies: 2, borrows: 2": "118594"
77
}

‎snapshots/Spoke.Operations.ZeroRiskPremium.json‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
{
2-
"borrow: first": "191325",
3-
"borrow: second action, same reserve": "171297",
4-
"liquidationCall (receiveShares): full": "300103",
5-
"liquidationCall (receiveShares): partial": "299821",
6-
"liquidationCall: full": "310468",
7-
"liquidationCall: partial": "310186",
8-
"permitReserve + repay (multicall)": "166029",
9-
"permitReserve + supply (multicall)": "146862",
10-
"permitReserve + supply + enable collateral (multicall)": "160573",
11-
"repay: full": "126094",
12-
"repay: partial": "130983",
2+
"borrow: first": "191801",
3+
"borrow: second action, same reserve": "171773",
4+
"liquidationCall (receiveShares): full": "300567",
5+
"liquidationCall (receiveShares): partial": "300285",
6+
"liquidationCall: full": "310932",
7+
"liquidationCall: partial": "310650",
8+
"permitReserve + repay (multicall)": "166498",
9+
"permitReserve + supply (multicall)": "147339",
10+
"permitReserve + supply + enable collateral (multicall)": "161516",
11+
"repay: full": "126563",
12+
"repay: partial": "131452",
1313
"setUserPositionManagerWithSig: disable": "44846",
1414
"setUserPositionManagerWithSig: enable": "68875",
15-
"supply + enable collateral (multicall)": "140624",
16-
"supply: 0 borrows, collateral disabled": "123679",
17-
"supply: 0 borrows, collateral enabled": "106601",
18-
"supply: second action, same reserve": "106579",
19-
"updateUserDynamicConfig: 1 collateral": "73694",
20-
"updateUserDynamicConfig: 2 collaterals": "88551",
21-
"updateUserRiskPremium: 1 borrow": "94804",
22-
"updateUserRiskPremium: 2 borrows": "104619",
23-
"usingAsCollateral: 0 borrows, enable": "58915",
24-
"usingAsCollateral: 1 borrow, disable": "105072",
25-
"usingAsCollateral: 1 borrow, enable": "41803",
26-
"usingAsCollateral: 2 borrows, disable": "126055",
27-
"usingAsCollateral: 2 borrows, enable": "41815",
28-
"withdraw: 0 borrows, full": "128910",
29-
"withdraw: 0 borrows, partial": "133473",
30-
"withdraw: 1 borrow, partial": "161036",
31-
"withdraw: 2 borrows, partial": "174214",
32-
"withdraw: non collateral": "106544"
15+
"supply + enable collateral (multicall)": "141567",
16+
"supply: 0 borrows, collateral disabled": "124156",
17+
"supply: 0 borrows, collateral enabled": "107078",
18+
"supply: second action, same reserve": "107056",
19+
"updateUserDynamicConfig: 1 collateral": "74160",
20+
"updateUserDynamicConfig: 2 collaterals": "89017",
21+
"updateUserRiskPremium: 1 borrow": "95269",
22+
"updateUserRiskPremium: 2 borrows": "105083",
23+
"usingAsCollateral: 0 borrows, enable": "59381",
24+
"usingAsCollateral: 1 borrow, disable": "105537",
25+
"usingAsCollateral: 1 borrow, enable": "42269",
26+
"usingAsCollateral: 2 borrows, disable": "126519",
27+
"usingAsCollateral: 2 borrows, enable": "42281",
28+
"withdraw: 0 borrows, full": "129379",
29+
"withdraw: 0 borrows, partial": "133942",
30+
"withdraw: 1 borrow, partial": "161504",
31+
"withdraw: 2 borrows, partial": "174681",
32+
"withdraw: non collateral": "107013"
3333
}

‎snapshots/Spoke.Operations.json‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
{
2-
"borrow: first": "261721",
3-
"borrow: second action, same reserve": "204693",
4-
"liquidationCall (receiveShares): full": "333666",
5-
"liquidationCall (receiveShares): partial": "333384",
6-
"liquidationCall: full": "344031",
7-
"liquidationCall: partial": "343749",
8-
"permitReserve + repay (multicall)": "163273",
9-
"permitReserve + supply (multicall)": "146862",
10-
"permitReserve + supply + enable collateral (multicall)": "160573",
11-
"repay: full": "120256",
12-
"repay: partial": "139545",
2+
"borrow: first": "262197",
3+
"borrow: second action, same reserve": "205169",
4+
"liquidationCall (receiveShares): full": "334130",
5+
"liquidationCall (receiveShares): partial": "333848",
6+
"liquidationCall: full": "344495",
7+
"liquidationCall: partial": "344213",
8+
"permitReserve + repay (multicall)": "163648",
9+
"permitReserve + supply (multicall)": "147339",
10+
"permitReserve + supply + enable collateral (multicall)": "161516",
11+
"repay: full": "120725",
12+
"repay: partial": "140014",
1313
"setUserPositionManagerWithSig: disable": "44846",
1414
"setUserPositionManagerWithSig: enable": "68875",
15-
"supply + enable collateral (multicall)": "140624",
16-
"supply: 0 borrows, collateral disabled": "123679",
17-
"supply: 0 borrows, collateral enabled": "106601",
18-
"supply: second action, same reserve": "106579",
19-
"updateUserDynamicConfig: 1 collateral": "73694",
20-
"updateUserDynamicConfig: 2 collaterals": "88551",
21-
"updateUserRiskPremium: 1 borrow": "151080",
22-
"updateUserRiskPremium: 2 borrows": "204276",
23-
"usingAsCollateral: 0 borrows, enable": "58915",
24-
"usingAsCollateral: 1 borrow, disable": "161348",
25-
"usingAsCollateral: 1 borrow, enable": "41803",
26-
"usingAsCollateral: 2 borrows, disable": "233712",
27-
"usingAsCollateral: 2 borrows, enable": "41815",
28-
"withdraw: 0 borrows, full": "128910",
29-
"withdraw: 0 borrows, partial": "133473",
30-
"withdraw: 1 borrow, partial": "214810",
31-
"withdraw: 2 borrows, partial": "259272",
32-
"withdraw: non collateral": "106544"
15+
"supply + enable collateral (multicall)": "141567",
16+
"supply: 0 borrows, collateral disabled": "124156",
17+
"supply: 0 borrows, collateral enabled": "107078",
18+
"supply: second action, same reserve": "107056",
19+
"updateUserDynamicConfig: 1 collateral": "74160",
20+
"updateUserDynamicConfig: 2 collaterals": "89017",
21+
"updateUserRiskPremium: 1 borrow": "151545",
22+
"updateUserRiskPremium: 2 borrows": "204740",
23+
"usingAsCollateral: 0 borrows, enable": "59381",
24+
"usingAsCollateral: 1 borrow, disable": "161813",
25+
"usingAsCollateral: 1 borrow, enable": "42269",
26+
"usingAsCollateral: 2 borrows, disable": "234176",
27+
"usingAsCollateral: 2 borrows, enable": "42281",
28+
"withdraw: 0 borrows, full": "129379",
29+
"withdraw: 0 borrows, partial": "133942",
30+
"withdraw: 1 borrow, partial": "215278",
31+
"withdraw: 2 borrows, partial": "259739",
32+
"withdraw: non collateral": "107013"
3333
}

‎src/spoke/Spoke.sol‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol';
66
import {SafeERC20, IERC20} from 'src/dependencies/openzeppelin/SafeERC20.sol';
77
import {IERC20Permit} from 'src/dependencies/openzeppelin/IERC20Permit.sol';
88
import {SignatureChecker} from 'src/dependencies/openzeppelin/SignatureChecker.sol';
9+
import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol';
910
import {AccessManagedUpgradeable} from 'src/dependencies/openzeppelin-upgradeable/AccessManagedUpgradeable.sol';
1011
import {EIP712} from 'src/dependencies/solady/EIP712.sol';
1112
import {MathUtils} from 'src/libraries/math/MathUtils.sol';
@@ -26,7 +27,14 @@ import {ISpokeBase, ISpoke} from 'src/spoke/interfaces/ISpoke.sol';
2627
/// @author Aave Labs
2728
/// @notice Handles risk configuration & borrowing strategy for reserves and user positions.
2829
/// @dev Each reserve can be associated with a separate Hub.
29-
abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradeable, EIP712 {
30+
abstract contract Spoke is
31+
ISpoke,
32+
Multicall,
33+
NoncesKeyed,
34+
AccessManagedUpgradeable,
35+
ReentrancyGuardTransient,
36+
EIP712
37+
{
3038
using SafeCast for *;
3139
using SafeERC20 for IERC20;
3240
using MathUtils for *;
@@ -231,7 +239,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
231239
uint256 reserveId,
232240
uint256 amount,
233241
address onBehalfOf
234-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
242+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
235243
Reserve storage reserve = _getReserve(reserveId);
236244
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
237245
_validateSupply(reserve.flags);
@@ -250,7 +258,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
250258
uint256 reserveId,
251259
uint256 amount,
252260
address onBehalfOf
253-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
261+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
254262
Reserve storage reserve = _getReserve(reserveId);
255263
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
256264
_validateWithdraw(reserve.flags);
@@ -280,7 +288,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
280288
uint256 reserveId,
281289
uint256 amount,
282290
address onBehalfOf
283-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
291+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
284292
Reserve storage reserve = _getReserve(reserveId);
285293
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
286294
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
@@ -306,7 +314,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
306314
uint256 reserveId,
307315
uint256 amount,
308316
address onBehalfOf
309-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
317+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
310318
Reserve storage reserve = _getReserve(reserveId);
311319
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
312320
_validateRepay(reserve.flags);
@@ -350,7 +358,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
350358
address user,
351359
uint256 debtToCover,
352360
bool receiveShares
353-
) external {
361+
) external nonReentrant {
354362
Reserve storage collateralReserve = _getReserve(collateralReserveId);
355363
Reserve storage debtReserve = _getReserve(debtReserveId);
356364
DynamicReserveConfig storage collateralDynConfig = _dynamicConfig[collateralReserveId][
@@ -404,7 +412,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
404412
uint256 reserveId,
405413
bool usingAsCollateral,
406414
address onBehalfOf
407-
) external onlyPositionManager(onBehalfOf) {
415+
) external nonReentrant onlyPositionManager(onBehalfOf) {
408416
_validateSetUsingAsCollateral(_getReserve(reserveId).flags, usingAsCollateral);
409417
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
410418

@@ -424,7 +432,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
424432
}
425433

426434
/// @inheritdoc ISpoke
427-
function updateUserRiskPremium(address onBehalfOf) external {
435+
function updateUserRiskPremium(address onBehalfOf) external nonReentrant {
428436
if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) {
429437
_checkCanCall(msg.sender, msg.data);
430438
}
@@ -433,7 +441,7 @@ abstract contract Spoke is ISpoke, Multicall, NoncesKeyed, AccessManagedUpgradea
433441
}
434442

435443
/// @inheritdoc ISpoke
436-
function updateUserDynamicConfig(address onBehalfOf) external {
444+
function updateUserDynamicConfig(address onBehalfOf) external nonReentrant {
437445
if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) {
438446
_checkCanCall(msg.sender, msg.data);
439447
}

‎tests/Base.t.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {console2 as console} from 'forge-std/console2.sol';
1212
// dependencies
1313
import {AggregatorV3Interface} from 'src/dependencies/chainlink/AggregatorV3Interface.sol';
1414
import {TransparentUpgradeableProxy, ITransparentUpgradeableProxy} from 'src/dependencies/openzeppelin/TransparentUpgradeableProxy.sol';
15+
import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol';
1516
import {IERC20Metadata} from 'src/dependencies/openzeppelin/IERC20Metadata.sol';
1617
import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol';
1718
import {IERC20Errors} from 'src/dependencies/openzeppelin/IERC20Errors.sol';
@@ -80,6 +81,7 @@ import {MockSpoke} from 'tests/mocks/MockSpoke.sol';
8081
import {MockERC1271Wallet} from 'tests/mocks/MockERC1271Wallet.sol';
8182
import {MockSpokeInstance} from 'tests/mocks/MockSpokeInstance.sol';
8283
import {MockSkimSpoke} from 'tests/mocks/MockSkimSpoke.sol';
84+
import {MockReentrantHub} from 'tests/mocks/MockReentrantHub.sol';
8385

8486
abstract contract Base is Test {
8587
using stdStorage for StdStorage;

‎tests/mocks/MockReentrantHub.sol‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
// Copyright (c) 2025 Aave Labs
3+
pragma solidity ^0.8.0;
4+
5+
import {Address} from 'src/dependencies/openzeppelin/Address.sol';
6+
7+
contract MockReentrantHub {
8+
using Address for address;
9+
10+
address public immutable TARGET;
11+
bytes4 public immutable TARGET_SELECTOR;
12+
13+
constructor(address target, bytes4 targetSelector) {
14+
TARGET = target;
15+
TARGET_SELECTOR = targetSelector;
16+
}
17+
18+
fallback() external {
19+
TARGET.functionCall(bytes.concat(TARGET_SELECTOR, new bytes(1000)));
20+
}
21+
}

0 commit comments

Comments
 (0)