Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: address axelar transceiver audit comments #22

Merged
merged 4 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/axelar/AxelarTransceiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract AxelarTransceiver is IAxelarTransceiver, AxelarGMPExecutable, Transceiv
mapping(uint16 => string) idToAxelarChainId;
mapping(string => uint16) axelarChainIdToId;
mapping(uint16 => string) idToTransceiverAddress;
mapping(string => uint16) transceiverAddressToId;
mapping(uint16 => bytes32) idToTransceiverAddressHash;
}
// keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.AxelarTransceiver")) - 1)) & ~bytes32(uint256(0xff))

Expand Down Expand Up @@ -79,11 +79,22 @@ contract AxelarTransceiver is IAxelarTransceiver, AxelarGMPExecutable, Transceiv
string calldata chainName,
string calldata transceiverAddress
) external virtual onlyOwner {
if (chainId == 0 || bytes(chainName).length == 0 || bytes(transceiverAddress).length == 0) {
revert InvalidChainIdParams();
}

AxelarTransceiverStorage storage slot = _storage();

if (bytes(slot.idToAxelarChainId[chainId]).length != 0) revert ChainIdAlreadySet(chainId);

if (slot.axelarChainIdToId[chainName] != 0) revert AxelarChainIdAlreadySet(chainName);
milapsheth marked this conversation as resolved.
Show resolved Hide resolved

slot.idToAxelarChainId[chainId] = chainName;
slot.axelarChainIdToId[chainName] = chainId;
slot.idToTransceiverAddress[chainId] = transceiverAddress;
slot.transceiverAddressToId[transceiverAddress] = chainId;
slot.idToTransceiverAddressHash[chainId] = keccak256(bytes(transceiverAddress));

emit AxelarChainIdSet(chainId, chainName, transceiverAddress);
}

/// @notice Fetch the delivery price for a given recipient chain transfer.
Expand Down Expand Up @@ -174,7 +185,10 @@ contract AxelarTransceiver is IAxelarTransceiver, AxelarGMPExecutable, Transceiv
) internal virtual override {
AxelarTransceiverStorage storage slot = _storage();
uint16 sourceChainId = slot.axelarChainIdToId[sourceChain];
if (sourceChainId == 0 || slot.transceiverAddressToId[sourceAddress] != sourceChainId) {
if (
sourceChainId == 0
|| slot.idToTransceiverAddressHash[sourceChainId] != keccak256(bytes(sourceAddress))
milapsheth marked this conversation as resolved.
Show resolved Hide resolved
) {
revert InvalidSibling(sourceChainId, sourceChain, sourceAddress);
}

Expand Down
19 changes: 18 additions & 1 deletion src/axelar/interfaces/IAxelarTransceiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,18 @@ interface IAxelarTransceiver is ITransceiver {

/// @notice Chain Id passed is not valid.
/// @param chainId The wormhole chainId.
error InvalidChainId(uint16 chainId, string chainName, string destinationContract);
/// @param chainName The axelar chainName.
/// @param transceiverAddress The address of the Transceiver as a string.
error InvalidChainId(uint16 chainId, string chainName, string transceiverAddress);

/// @notice Chain Id passed is zero, or Axelar Chain Id or Transceiver Address were empty.
error InvalidChainIdParams();

/// @notice Chain Id is already being used.
error ChainIdAlreadySet(uint16 chainId);

/// @notice Axelar chain Id is already being used.
error AxelarChainIdAlreadySet(string axelarChainId);

/// @notice Emmited when a transceiver message is sent.
/// @param recipientChainId The wormhole chainId of the destination chain.
Expand All @@ -26,6 +37,12 @@ interface IAxelarTransceiver is ITransceiver {
bytes32 indexed refundAddress
);

/// @notice Emmited when the chain id is set.
/// @param chainId The wormhole chainId of the destination chain.
/// @param chainName The axelar chain name.
/// @param transceiverAddress The transceiver address as a string.
event AxelarChainIdSet(uint16 chainId, string chainName, string transceiverAddress);

/**
* Set the bridge manager contract address
* @param chainId The chainId of the chain. This is used to identify the chain in the EndpointManager.
Expand Down
64 changes: 53 additions & 11 deletions test/axelar/AxelarTransceiver.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ import "forge-std/Test.sol";
import "openzeppelin-contracts/contracts/access/Ownable.sol";

contract AxelarTransceiverTest is Test {
event ContractCall(
address indexed sender,
string destinationChain,
string destinationContractAddress,
bytes32 indexed payloadHash,
bytes payload
);

event AxelarChainIdSet(uint16 chainId, string chainName, string transceiverAddress);

address constant OWNER = address(1004);

uint64 constant RATE_LIMIT_DURATION = 0;
Expand All @@ -34,9 +44,6 @@ contract AxelarTransceiverTest is Test {
WstEthL2TokenHarness token;

function setUp() public {
string memory url = "https://ethereum-sepolia-rpc.publicnode.com";
vm.createSelectFork(url);

gateway = IAxelarGateway(new MockAxelarGateway());
gasService = IAxelarGasService(address(new MockAxelarGasService()));

Expand Down Expand Up @@ -75,12 +82,31 @@ contract AxelarTransceiverTest is Test {
string memory chainName = "chainName";
string memory axelarAddress = "axelarAddress";

vm.expectEmit(address(transceiver));
emit AxelarChainIdSet(chainId, chainName, axelarAddress);

vm.prank(OWNER);
transceiver.setAxelarChainId(chainId, chainName, axelarAddress);
/*assertEq(transceiver.idToAxelarChainIds(chainId), chainName);
assertEq(transceiver.axelarChainIdToId(chainName),chainId);
assertEq(transceiver.idToAxelarAddress(chainId), axelarAddress);
assertEq(transceiver.axelarAddressToId(axelarAddress), chainId);*/
}

function test_setAxelarChainIdDuplicateChainId() public {
uint16 chainId = 1;
string memory chainName = "chainName";
string memory axelarAddress = "axelarAddress";

vm.expectEmit(address(transceiver));
emit AxelarChainIdSet(chainId, chainName, axelarAddress);

vm.prank(OWNER);
transceiver.setAxelarChainId(chainId, chainName, axelarAddress);

vm.prank(OWNER);
vm.expectRevert(abi.encodeWithSignature("ChainIdAlreadySet(uint16)", chainId));
transceiver.setAxelarChainId(chainId, chainName, axelarAddress);

vm.prank(OWNER);
vm.expectRevert(abi.encodeWithSignature("AxelarChainIdAlreadySet(string)", chainName));
transceiver.setAxelarChainId(chainId + 1, chainName, axelarAddress);
}

function test_setAxelarChainIdNotOwner() public {
Expand All @@ -101,11 +127,18 @@ contract AxelarTransceiverTest is Test {
bytes32 recipientNttManagerAddress = bytes32(uint256(1010));
bytes memory nttManagerMessage = bytes("nttManagerMessage");
bytes32 refundAddress = bytes32(uint256(1011));
bytes memory payload = abi.encode(manager, nttManagerMessage, recipientNttManagerAddress);
TransceiverStructs.TransceiverInstruction memory instruction =
TransceiverStructs.TransceiverInstruction(0, bytes(""));

vm.prank(OWNER);
transceiver.setAxelarChainId(chainId, chainName, axelarAddress);

vm.expectEmit(address(gateway));
emit ContractCall(
address(transceiver), chainName, axelarAddress, keccak256(payload), payload
);

vm.prank(address(manager));
transceiver.sendMessage(
chainId, instruction, nttManagerMessage, recipientNttManagerAddress, refundAddress
Expand Down Expand Up @@ -167,6 +200,7 @@ contract AxelarTransceiverTest is Test {
uint16 chainId = 2;
string memory chainName = "chainName";
string memory axelarAddress = "axelarAddress";
bytes32 messageId = bytes32(uint256(25));
bytes32 recipientNttManagerAddress = bytes32(uint256(uint160(address(manager))));

bytes32 to = bytes32(uint256(1234));
Expand All @@ -184,9 +218,9 @@ contract AxelarTransceiverTest is Test {
bytes memory nttManagerMessage;
{
uint16 length = uint16(nttPayload.length);
bytes32 messageId = bytes32(uint256(0));
bytes32 nttMessageId = bytes32(uint256(0));
bytes32 sender = bytes32(uint256(1));
nttManagerMessage = abi.encodePacked(messageId, sender, length, nttPayload);
nttManagerMessage = abi.encodePacked(nttMessageId, sender, length, nttPayload);
}

bytes32 sourceNttManagerAddress = bytes32(uint256(1012));
Expand All @@ -201,21 +235,29 @@ contract AxelarTransceiverTest is Test {
token.setMinter(OWNER);
vm.prank(OWNER);
token.mint(address(manager), amount);
gateway.approveContractCall(messageId, chainName, axelarAddress, keccak256(payload));

transceiver.execute(bytes32(0), chainName, axelarAddress, payload);
transceiver.execute(messageId, chainName, axelarAddress, payload);

if (token.balanceOf(fromWormholeFormat(to)) != amount) revert("Amount Incorrect");

vm.prank(OWNER);
token.mint(address(manager), amount);
vm.expectRevert(abi.encodeWithSignature("NotApprovedByGateway()"));
transceiver.execute(bytes32(0), chainName, axelarAddress, payload);
}

function test_executeNotTrustedAddress() public {
string memory chainName = "chainName";
string memory axelarAddress = "axelarAddress";
bytes memory payload = bytes("");
bytes32 messageId = keccak256(bytes("message Id"));
gateway.approveContractCall(messageId, chainName, axelarAddress, keccak256(payload));
vm.expectRevert(
abi.encodeWithSignature(
"InvalidSibling(uint16,string,string)", 0, chainName, axelarAddress
)
);
transceiver.execute(bytes32(0), chainName, axelarAddress, payload);
transceiver.execute(messageId, chainName, axelarAddress, payload);
}
}
Loading