Skip to content

Commit ab9d1cb

Browse files
avniculaeavniculae
authored
feat: Add reentrancy locks in Spoke and NativeTokenGateway (#1073)
1 parent 174b759 commit ab9d1cb

17 files changed

+651
-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": "226775",
3-
"repayNative": "165738",
4-
"supplyAsCollateralNative": "159246",
5-
"supplyNative": "135381",
6-
"withdrawNative: full": "124624",
7-
"withdrawNative: partial": "135579"
2+
"borrowNative": "227643",
3+
"repayNative": "166113",
4+
"supplyAsCollateralNative": "159972",
5+
"supplyNative": "135681",
6+
"withdrawNative: full": "125318",
7+
"withdrawNative: partial": "136447"
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": "212528",
3-
"repayWithSig": "186080",
2+
"borrowWithSig": "212903",
3+
"repayWithSig": "186455",
44
"setSelfAsUserPositionManagerWithSig": "74852",
5-
"setUsingAsCollateralWithSig": "85012",
6-
"supplyWithSig": "151642",
7-
"updateUserDynamicConfigWithSig": "62750",
8-
"updateUserRiskPremiumWithSig": "61732",
9-
"withdrawWithSig": "130328"
5+
"setUsingAsCollateralWithSig": "85363",
6+
"supplyWithSig": "151942",
7+
"updateUserDynamicConfigWithSig": "63101",
8+
"updateUserRiskPremiumWithSig": "62083",
9+
"withdrawWithSig": "130628"
1010
}

‎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": "188895",
3-
"borrow: second action, same reserve": "168844",
4-
"liquidationCall (receiveShares): full": "294611",
5-
"liquidationCall (receiveShares): partial": "294329",
6-
"liquidationCall: full": "303896",
7-
"liquidationCall: partial": "303614",
8-
"permitReserve + repay (multicall)": "164002",
9-
"permitReserve + supply (multicall)": "146269",
10-
"permitReserve + supply + enable collateral (multicall)": "160321",
11-
"repay: full": "123339",
12-
"repay: partial": "128309",
2+
"borrow: first": "189270",
3+
"borrow: second action, same reserve": "169219",
4+
"liquidationCall (receiveShares): full": "294986",
5+
"liquidationCall (receiveShares): partial": "294704",
6+
"liquidationCall: full": "304271",
7+
"liquidationCall: partial": "303989",
8+
"permitReserve + repay (multicall)": "164377",
9+
"permitReserve + supply (multicall)": "146644",
10+
"permitReserve + supply + enable collateral (multicall)": "161047",
11+
"repay: full": "123714",
12+
"repay: partial": "128684",
1313
"setUserPositionManagerWithSig: disable": "44889",
1414
"setUserPositionManagerWithSig: enable": "68930",
15-
"supply + enable collateral (multicall)": "140501",
16-
"supply: 0 borrows, collateral disabled": "122370",
17-
"supply: 0 borrows, collateral enabled": "105341",
18-
"supply: second action, same reserve": "105270",
19-
"updateUserDynamicConfig: 1 collateral": "74176",
20-
"updateUserDynamicConfig: 2 collaterals": "89081",
21-
"updateUserRiskPremium: 1 borrow": "94409",
22-
"updateUserRiskPremium: 2 borrows": "103544",
23-
"usingAsCollateral: 0 borrows, enable": "59205",
24-
"usingAsCollateral: 1 borrow, disable": "104524",
25-
"usingAsCollateral: 1 borrow, enable": "42093",
26-
"usingAsCollateral: 2 borrows, disable": "125620",
27-
"usingAsCollateral: 2 borrows, enable": "42105",
28-
"withdraw: 0 borrows, full": "127285",
29-
"withdraw: 0 borrows, partial": "131971",
30-
"withdraw: 1 borrow, partial": "158353",
31-
"withdraw: 2 borrows, partial": "172459",
32-
"withdraw: non collateral": "105240"
15+
"supply + enable collateral (multicall)": "141227",
16+
"supply: 0 borrows, collateral disabled": "122745",
17+
"supply: 0 borrows, collateral enabled": "105716",
18+
"supply: second action, same reserve": "105645",
19+
"updateUserDynamicConfig: 1 collateral": "74527",
20+
"updateUserDynamicConfig: 2 collaterals": "89432",
21+
"updateUserRiskPremium: 1 borrow": "94760",
22+
"updateUserRiskPremium: 2 borrows": "103895",
23+
"usingAsCollateral: 0 borrows, enable": "59556",
24+
"usingAsCollateral: 1 borrow, disable": "104875",
25+
"usingAsCollateral: 1 borrow, enable": "42444",
26+
"usingAsCollateral: 2 borrows, disable": "125971",
27+
"usingAsCollateral: 2 borrows, enable": "42456",
28+
"withdraw: 0 borrows, full": "127660",
29+
"withdraw: 0 borrows, partial": "132346",
30+
"withdraw: 1 borrow, partial": "158728",
31+
"withdraw: 2 borrows, partial": "172834",
32+
"withdraw: non collateral": "105615"
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": "257798",
3-
"borrow: second action, same reserve": "200747",
4-
"liquidationCall (receiveShares): full": "326619",
5-
"liquidationCall (receiveShares): partial": "326337",
6-
"liquidationCall: full": "335904",
7-
"liquidationCall: partial": "335622",
8-
"permitReserve + repay (multicall)": "161585",
9-
"permitReserve + supply (multicall)": "146269",
10-
"permitReserve + supply + enable collateral (multicall)": "160321",
11-
"repay: full": "117418",
12-
"repay: partial": "136788",
2+
"borrow: first": "258173",
3+
"borrow: second action, same reserve": "201122",
4+
"liquidationCall (receiveShares): full": "326994",
5+
"liquidationCall (receiveShares): partial": "326712",
6+
"liquidationCall: full": "336279",
7+
"liquidationCall: partial": "335997",
8+
"permitReserve + repay (multicall)": "161885",
9+
"permitReserve + supply (multicall)": "146644",
10+
"permitReserve + supply + enable collateral (multicall)": "161047",
11+
"repay: full": "117793",
12+
"repay: partial": "137163",
1313
"setUserPositionManagerWithSig: disable": "44889",
1414
"setUserPositionManagerWithSig: enable": "68930",
15-
"supply + enable collateral (multicall)": "140501",
16-
"supply: 0 borrows, collateral disabled": "122370",
17-
"supply: 0 borrows, collateral enabled": "105341",
18-
"supply: second action, same reserve": "105270",
19-
"updateUserDynamicConfig: 1 collateral": "74176",
20-
"updateUserDynamicConfig: 2 collaterals": "89081",
21-
"updateUserRiskPremium: 1 borrow": "147733",
22-
"updateUserRiskPremium: 2 borrows": "197440",
23-
"usingAsCollateral: 0 borrows, enable": "59205",
24-
"usingAsCollateral: 1 borrow, disable": "157848",
25-
"usingAsCollateral: 1 borrow, enable": "42093",
26-
"usingAsCollateral: 2 borrows, disable": "227516",
27-
"usingAsCollateral: 2 borrows, enable": "42105",
28-
"withdraw: 0 borrows, full": "127285",
29-
"withdraw: 0 borrows, partial": "131971",
30-
"withdraw: 1 borrow, partial": "209175",
31-
"withdraw: 2 borrows, partial": "254888",
32-
"withdraw: non collateral": "105240"
15+
"supply + enable collateral (multicall)": "141227",
16+
"supply: 0 borrows, collateral disabled": "122745",
17+
"supply: 0 borrows, collateral enabled": "105716",
18+
"supply: second action, same reserve": "105645",
19+
"updateUserDynamicConfig: 1 collateral": "74527",
20+
"updateUserDynamicConfig: 2 collaterals": "89432",
21+
"updateUserRiskPremium: 1 borrow": "148084",
22+
"updateUserRiskPremium: 2 borrows": "197791",
23+
"usingAsCollateral: 0 borrows, enable": "59556",
24+
"usingAsCollateral: 1 borrow, disable": "158199",
25+
"usingAsCollateral: 1 borrow, enable": "42444",
26+
"usingAsCollateral: 2 borrows, disable": "227867",
27+
"usingAsCollateral: 2 borrows, enable": "42456",
28+
"withdraw: 0 borrows, full": "127660",
29+
"withdraw: 0 borrows, partial": "132346",
30+
"withdraw: 1 borrow, partial": "209550",
31+
"withdraw: 2 borrows, partial": "255263",
32+
"withdraw: non collateral": "105615"
3333
}

‎src/position-manager/NativeTokenGateway.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ contract NativeTokenGateway is INativeTokenGateway, GatewayBase, ReentrancyGuard
7070
address spoke,
7171
uint256 reserveId,
7272
uint256 amount
73-
) external onlyRegisteredSpoke(spoke) returns (uint256, uint256) {
73+
) external nonReentrant onlyRegisteredSpoke(spoke) returns (uint256, uint256) {
7474
address underlying = _getReserveUnderlying(spoke, reserveId);
7575
_validateParams(underlying, amount);
7676

@@ -90,7 +90,7 @@ contract NativeTokenGateway is INativeTokenGateway, GatewayBase, ReentrancyGuard
9090
address spoke,
9191
uint256 reserveId,
9292
uint256 amount
93-
) external onlyRegisteredSpoke(spoke) returns (uint256, uint256) {
93+
) external nonReentrant onlyRegisteredSpoke(spoke) returns (uint256, uint256) {
9494
address underlying = _getReserveUnderlying(spoke, reserveId);
9595
_validateParams(underlying, amount);
9696

‎src/spoke/Spoke.sol‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ pragma solidity 0.8.28;
55
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';
8+
import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol';
89
import {AccessManagedUpgradeable} from 'src/dependencies/openzeppelin-upgradeable/AccessManagedUpgradeable.sol';
910
import {MathUtils} from 'src/libraries/math/MathUtils.sol';
1011
import {PercentageMath} from 'src/libraries/math/PercentageMath.sol';
@@ -24,7 +25,13 @@ import {ISpokeBase, ISpoke} from 'src/spoke/interfaces/ISpoke.sol';
2425
/// @author Aave Labs
2526
/// @notice Handles risk configuration & borrowing strategy for reserves and user positions.
2627
/// @dev Each reserve can be associated with a separate Hub.
27-
abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Multicall {
28+
abstract contract Spoke is
29+
ISpoke,
30+
AccessManagedUpgradeable,
31+
IntentConsumer,
32+
Multicall,
33+
ReentrancyGuardTransient
34+
{
2835
using SafeCast for *;
2936
using SafeERC20 for IERC20;
3037
using MathUtils for *;
@@ -229,7 +236,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
229236
uint256 reserveId,
230237
uint256 amount,
231238
address onBehalfOf
232-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
239+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
233240
Reserve storage reserve = _getReserve(reserveId);
234241
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
235242
_validateSupply(reserve.flags);
@@ -248,7 +255,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
248255
uint256 reserveId,
249256
uint256 amount,
250257
address onBehalfOf
251-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
258+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
252259
Reserve storage reserve = _getReserve(reserveId);
253260
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
254261
_validateWithdraw(reserve.flags);
@@ -278,7 +285,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
278285
uint256 reserveId,
279286
uint256 amount,
280287
address onBehalfOf
281-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
288+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
282289
Reserve storage reserve = _getReserve(reserveId);
283290
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
284291
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
@@ -304,7 +311,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
304311
uint256 reserveId,
305312
uint256 amount,
306313
address onBehalfOf
307-
) external onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
314+
) external nonReentrant onlyPositionManager(onBehalfOf) returns (uint256, uint256) {
308315
Reserve storage reserve = _getReserve(reserveId);
309316
UserPosition storage userPosition = _userPositions[onBehalfOf][reserveId];
310317
_validateRepay(reserve.flags);
@@ -348,7 +355,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
348355
address user,
349356
uint256 debtToCover,
350357
bool receiveShares
351-
) external {
358+
) external nonReentrant {
352359
Reserve storage collateralReserve = _getReserve(collateralReserveId);
353360
Reserve storage debtReserve = _getReserve(debtReserveId);
354361
DynamicReserveConfig storage collateralDynConfig = _dynamicConfig[collateralReserveId][
@@ -402,7 +409,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
402409
uint256 reserveId,
403410
bool usingAsCollateral,
404411
address onBehalfOf
405-
) external onlyPositionManager(onBehalfOf) {
412+
) external nonReentrant onlyPositionManager(onBehalfOf) {
406413
_validateSetUsingAsCollateral(_getReserve(reserveId).flags, usingAsCollateral);
407414
PositionStatus storage positionStatus = _positionStatus[onBehalfOf];
408415

@@ -422,7 +429,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
422429
}
423430

424431
/// @inheritdoc ISpoke
425-
function updateUserRiskPremium(address onBehalfOf) external {
432+
function updateUserRiskPremium(address onBehalfOf) external nonReentrant {
426433
if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) {
427434
_checkCanCall(msg.sender, msg.data);
428435
}
@@ -431,7 +438,7 @@ abstract contract Spoke is ISpoke, AccessManagedUpgradeable, IntentConsumer, Mul
431438
}
432439

433440
/// @inheritdoc ISpoke
434-
function updateUserDynamicConfig(address onBehalfOf) external {
441+
function updateUserDynamicConfig(address onBehalfOf) external nonReentrant {
435442
if (!_isPositionManager({user: onBehalfOf, manager: msg.sender})) {
436443
_checkCanCall(msg.sender, msg.data);
437444
}

‎tests/Base.t.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
TransparentUpgradeableProxy,
1616
ITransparentUpgradeableProxy
1717
} from 'src/dependencies/openzeppelin/TransparentUpgradeableProxy.sol';
18+
import {ReentrancyGuardTransient} from 'src/dependencies/openzeppelin/ReentrancyGuardTransient.sol';
1819
import {IERC20Metadata} from 'src/dependencies/openzeppelin/IERC20Metadata.sol';
1920
import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol';
2021
import {IERC20Errors} from 'src/dependencies/openzeppelin/IERC20Errors.sol';
@@ -88,6 +89,7 @@ import {MockSpoke} from 'tests/mocks/MockSpoke.sol';
8889
import {MockERC1271Wallet} from 'tests/mocks/MockERC1271Wallet.sol';
8990
import {MockSpokeInstance} from 'tests/mocks/MockSpokeInstance.sol';
9091
import {MockSkimSpoke} from 'tests/mocks/MockSkimSpoke.sol';
92+
import {MockReentrantCaller} from 'tests/mocks/MockReentrantCaller.sol';
9193
import {ISpokeInstance} from 'tests/mocks/ISpokeInstance.sol';
9294
import {DeployWrapper} from 'tests/mocks/DeployWrapper.sol';
9395

‎tests/mocks/MockReentrantCaller.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 MockReentrantCaller {
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)