Skip to content

Commit

Permalink
security-enhance: add ReentrancyGuard to contracts (#35)
Browse files Browse the repository at this point in the history
* security-enhance: add reentrancyguard to contracts and NonReentrant modifier to user facing functions

* fix format and lint
  • Loading branch information
adu-web3 authored Jul 1, 2024
1 parent 74cd9b6 commit 26c3755
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 7 deletions.
5 changes: 5 additions & 0 deletions src/core/BaseRestakingController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import {ClientChainGatewayStorage} from "../storage/ClientChainGatewayStorage.so

import {OptionsBuilder} from "@layerzero-v2/oapp/contracts/oapp/libs/OptionsBuilder.sol";
import {PausableUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/PausableUpgradeable.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol";

abstract contract BaseRestakingController is
PausableUpgradeable,
ReentrancyGuardUpgradeable,
OAppSenderUpgradeable,
IBaseRestakingController,
ClientChainGatewayStorage
Expand All @@ -25,6 +27,7 @@ abstract contract BaseRestakingController is
isTokenWhitelisted(token)
isValidAmount(amount)
whenNotPaused
nonReentrant
{
if (token == VIRTUAL_STAKED_ETH_ADDRESS) {
IExoCapsule capsule = _getCapsule(msg.sender);
Expand All @@ -44,6 +47,7 @@ abstract contract BaseRestakingController is
isValidAmount(amount)
isValidBech32Address(operator)
whenNotPaused
nonReentrant
{
bytes memory actionArgs =
abi.encodePacked(bytes32(bytes20(token)), bytes32(bytes20(msg.sender)), bytes(operator), amount);
Expand All @@ -58,6 +62,7 @@ abstract contract BaseRestakingController is
isValidAmount(amount)
isValidBech32Address(operator)
whenNotPaused
nonReentrant
{
bytes memory actionArgs =
abi.encodePacked(bytes32(bytes20(token)), bytes32(bytes20(msg.sender)), bytes(operator), amount);
Expand Down
1 change: 1 addition & 0 deletions src/core/Bootstrap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ contract Bootstrap is
// contract controlled by one.
__Ownable_init_unchained(owner);
__Pausable_init_unchained();
__ReentrancyGuard_init_unchained();
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/core/ClientChainGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ contract ClientChainGateway is
__Ownable_init_unchained(exocoreValidatorSetAddress);
__OAppCore_init_unchained(exocoreValidatorSetAddress);
__Pausable_init_unchained();
__ReentrancyGuard_init_unchained();
}

function _clearBootstrapData() internal {
Expand Down Expand Up @@ -119,7 +120,7 @@ contract ClientChainGateway is
}

// implementation of ITokenWhitelister
function addWhitelistTokens(address[] calldata tokens) external payable onlyOwner whenNotPaused {
function addWhitelistTokens(address[] calldata tokens) external payable onlyOwner whenNotPaused nonReentrant {
_addWhitelistTokens(tokens);
}

Expand Down
13 changes: 11 additions & 2 deletions src/core/ExocoreGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import {ILayerZeroReceiver} from "@layerzero-v2/protocol/contracts/interfaces/IL
import {OwnableUpgradeable} from "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {Initializable} from "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol";
import {PausableUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/PausableUpgradeable.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol";

contract ExocoreGateway is
Initializable,
PausableUpgradeable,
OwnableUpgradeable,
ReentrancyGuardUpgradeable,
IExocoreGateway,
ExocoreGatewayStorage,
OAppUpgradeable
Expand Down Expand Up @@ -60,6 +62,7 @@ contract ExocoreGateway is
__Ownable_init_unchained(exocoreValidatorSetAddress);
__OAppCore_init_unchained(exocoreValidatorSetAddress);
__Pausable_init_unchained();
__ReentrancyGuard_init_unchained();
}

function _initializeWhitelistFunctionSelectors() private {
Expand Down Expand Up @@ -94,7 +97,7 @@ contract ExocoreGateway is
// setPeer) or be triggered by Golang after the contract is deployed.
// For manual calls, this function should be called immediately after deployment and
// then never needs to be called again.
function markBootstrapOnAllChains() public whenNotPaused {
function markBootstrapOnAllChains() public whenNotPaused nonReentrant {
(bool success, bytes memory result) =
ASSETS_PRECOMPILE_ADDRESS.staticcall(abi.encodeWithSelector(ASSETS_CONTRACT.getClientChains.selector));
require(success, "ExocoreGateway: failed to get client chain ids");
Expand Down Expand Up @@ -145,7 +148,13 @@ contract ExocoreGateway is
}
}

function _lzReceive(Origin calldata _origin, bytes calldata payload) internal virtual override whenNotPaused {
function _lzReceive(Origin calldata _origin, bytes calldata payload)
internal
virtual
override
whenNotPaused
nonReentrant
{
_verifyAndUpdateNonce(_origin.srcEid, _origin.sender, _origin.nonce);

Action act = Action(uint8(payload[0]));
Expand Down
12 changes: 11 additions & 1 deletion src/core/LSTRestakingController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,22 @@ import {IVault} from "../interfaces/IVault.sol";
import {BaseRestakingController} from "./BaseRestakingController.sol";

import {PausableUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/PausableUpgradeable.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol";

abstract contract LSTRestakingController is PausableUpgradeable, ILSTRestakingController, BaseRestakingController {
abstract contract LSTRestakingController is
PausableUpgradeable,
ReentrancyGuardUpgradeable,
ILSTRestakingController,
BaseRestakingController
{

function deposit(address token, uint256 amount)
external
payable
isTokenWhitelisted(token)
isValidAmount(amount)
whenNotPaused
nonReentrant
{
IVault vault = _getVault(token);
vault.deposit(msg.sender, amount);
Expand All @@ -31,6 +38,7 @@ abstract contract LSTRestakingController is PausableUpgradeable, ILSTRestakingCo
isTokenWhitelisted(token)
isValidAmount(principalAmount)
whenNotPaused
nonReentrant
{
bytes memory actionArgs =
abi.encodePacked(bytes32(bytes20(token)), bytes32(bytes20(msg.sender)), principalAmount);
Expand All @@ -45,6 +53,7 @@ abstract contract LSTRestakingController is PausableUpgradeable, ILSTRestakingCo
isTokenWhitelisted(token)
isValidAmount(rewardAmount)
whenNotPaused
nonReentrant
{
bytes memory actionArgs = abi.encodePacked(bytes32(bytes20(token)), bytes32(bytes20(msg.sender)), rewardAmount);
bytes memory encodedRequest = abi.encode(token, msg.sender, rewardAmount);
Expand All @@ -60,6 +69,7 @@ abstract contract LSTRestakingController is PausableUpgradeable, ILSTRestakingCo
isValidAmount(amount)
isValidBech32Address(operator)
whenNotPaused
nonReentrant
{
IVault vault = _getVault(token);
vault.deposit(msg.sender, amount);
Expand Down
9 changes: 6 additions & 3 deletions src/core/NativeRestakingController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import {BaseRestakingController} from "./BaseRestakingController.sol";
import {ExoCapsule} from "./ExoCapsule.sol";

import {PausableUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/PausableUpgradeable.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol";
import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";

abstract contract NativeRestakingController is
PausableUpgradeable,
ReentrancyGuardUpgradeable,
INativeRestakingController,
BaseRestakingController
{
Expand All @@ -22,6 +24,7 @@ abstract contract NativeRestakingController is
external
payable
whenNotPaused
nonReentrant
{
require(msg.value == 32 ether, "NativeRestakingController: stake value must be exactly 32 ether");

Expand Down Expand Up @@ -58,7 +61,7 @@ abstract contract NativeRestakingController is
function depositBeaconChainValidator(
bytes32[] calldata validatorContainer,
IExoCapsule.ValidatorContainerProof calldata proof
) external payable whenNotPaused {
) external payable whenNotPaused nonReentrant {
IExoCapsule capsule = _getCapsule(msg.sender);
capsule.verifyDepositProof(validatorContainer, proof);

Expand All @@ -75,13 +78,13 @@ abstract contract NativeRestakingController is
IExoCapsule.ValidatorContainerProof calldata validatorProof,
bytes32[] calldata withdrawalContainer,
IExoCapsule.WithdrawalContainerProof calldata withdrawalProof
) external payable whenNotPaused {}
) external payable whenNotPaused nonReentrant {}

function processBeaconChainFullWithdrawal(
bytes32[] calldata validatorContainer,
IExoCapsule.ValidatorContainerProof calldata validatorProof,
bytes32[] calldata withdrawalContainer,
IExoCapsule.WithdrawalContainerProof calldata withdrawalProof
) external payable whenNotPaused {}
) external payable whenNotPaused nonReentrant {}

}

0 comments on commit 26c3755

Please sign in to comment.