Skip to content

Commit

Permalink
validate constructor args
Browse files Browse the repository at this point in the history
  • Loading branch information
ezynda3 committed Feb 24, 2025
1 parent f4bce45 commit 0778cc0
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 3 deletions.
5 changes: 4 additions & 1 deletion src/Facets/ChainflipFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { SwapperV2 } from "../Helpers/SwapperV2.sol";
import { Validatable } from "../Helpers/Validatable.sol";
import { IChainflipVault } from "../Interfaces/IChainflip.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { InformationMismatch } from "../Errors/GenericErrors.sol";
import { InformationMismatch, InvalidConfig } from "../Errors/GenericErrors.sol";

/// @title Chainflip Facet
/// @author LI.FI (https://li.fi)
Expand Down Expand Up @@ -62,6 +62,9 @@ contract ChainflipFacet is ILiFi, ReentrancyGuard, SwapperV2, Validatable {
/// @notice Constructor for the contract.
/// @param _chainflipVault Address of the Chainflip vault contract
constructor(IChainflipVault _chainflipVault) {

Check notice

Code scanning / Olympix Integrated Security

Test functions fail to thoroughly test all aspects of contract constructors, potentially missing critical initialization issues. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/incomplete-constructor-tests Low

Test functions fail to thoroughly test all aspects of contract constructors, potentially missing critical initialization issues. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/incomplete-constructor-tests
if (address(_chainflipVault) == address(0)) {
revert InvalidConfig();
}
chainflipVault = _chainflipVault;
}

Expand Down
8 changes: 8 additions & 0 deletions src/Periphery/ReceiverChainflip.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IExecutor } from "../Interfaces/IExecutor.sol";
import { WithdrawablePeriphery } from "../Helpers/WithdrawablePeriphery.sol";
import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
import { console2 } from "forge-std/console2.sol";
import { InvalidConfig } from "../Errors/GenericErrors.sol";

/// @title ReceiverChainflip
/// @author LI.FI (https://li.fi)
Expand Down Expand Up @@ -49,6 +50,13 @@ contract ReceiverChainflip is ILiFi, WithdrawablePeriphery {
address _executor,
address _chainflipVault
) WithdrawablePeriphery(_owner) {
if (
_owner == address(0) ||
_executor == address(0) ||
_chainflipVault == address(0)
) {
revert InvalidConfig();
}
executor = IExecutor(_executor);
chainflipVault = _chainflipVault;
}
Expand Down
7 changes: 6 additions & 1 deletion test/solidity/Facets/ChainflipFacet.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ChainflipFacet } from "lifi/Facets/ChainflipFacet.sol";
import { IChainflipVault } from "lifi/Interfaces/IChainflip.sol";
import { LibAsset } from "lifi/Libraries/LibAsset.sol";
import { LibSwap } from "lifi/Libraries/LibSwap.sol";
import { InformationMismatch, CannotBridgeToSameNetwork } from "lifi/Errors/GenericErrors.sol";
import { InformationMismatch, CannotBridgeToSameNetwork, InvalidConfig } from "lifi/Errors/GenericErrors.sol";

// Stub ChainflipFacet Contract
contract TestChainflipFacet is ChainflipFacet {
Expand Down Expand Up @@ -82,6 +82,11 @@ contract ChainflipFacetTest is TestBaseFacet {
validChainflipData.dstToken = 7;
}

function testRevert_WhenConstructedWithZeroAddress() public {
vm.expectRevert(InvalidConfig.selector);
new TestChainflipFacet(address(0));
}

function initiateBridgeTxWithFacet(bool isNative) internal override {
if (isNative) {
chainflipFacet.startBridgeTokensViaChainflip{
Expand Down
18 changes: 17 additions & 1 deletion test/solidity/Periphery/ReceiverChainflip.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.17;

import { TestBase, ILiFi, LibSwap, console, ERC20, UniswapV2Router02 } from "../utils/TestBase.sol";
import { ExternalCallFailed, UnAuthorized } from "src/Errors/GenericErrors.sol";
import { ExternalCallFailed, UnAuthorized, InvalidConfig } from "src/Errors/GenericErrors.sol";
import { ReceiverChainflip } from "lifi/Periphery/ReceiverChainflip.sol";
import { LibAsset } from "lifi/Libraries/LibAsset.sol";
import { stdJson } from "forge-std/Script.sol";
Expand All @@ -27,6 +27,20 @@ contract ReceiverChainflipTest is TestBase {

event ExecutorSet(address indexed executor);

function testRevert_WhenConstructedWithZeroAddresses() public {
// Test zero owner address
vm.expectRevert(InvalidConfig.selector);
new ReceiverChainflip(address(0), address(executor), chainflipVault);

// Test zero executor address
vm.expectRevert(InvalidConfig.selector);
new ReceiverChainflip(address(this), address(0), chainflipVault);

// Test zero chainflip vault address
vm.expectRevert(InvalidConfig.selector);
new ReceiverChainflip(address(this), address(executor), address(0));
}

function setUp() public {
customBlockNumberForForking = 18277082;
initTestBase();
Expand All @@ -49,6 +63,8 @@ contract ReceiverChainflipTest is TestBase {
vm.label(address(erc20Proxy), "ERC20Proxy");
}

// AI! add a test that makes sure we revert when constructing with zero address. Check each arg. Make sure to import InvalidConfig from GenericErrors

function test_ContractIsSetUpCorrectly() public {
receiver = new ReceiverChainflip(
address(this),
Expand Down

0 comments on commit 0778cc0

Please sign in to comment.