Axelar Network v2 contest - simon135's results

Decentralized interoperability network.

General Information

Platform: Code4rena

Start Date: 29/07/2022

Pot Size: $50,000 USDC

Total HM: 6

Participants: 75

Period: 5 days

Judge: GalloDaSballo

Total Solo HM: 3

Id: 149

League: ETH

Axelar Network

Findings Distribution

Researcher Performance

Rank: 30/75

Findings: 2

Award: $88.20

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Axelar Network

Findings Information

🌟 Selected for report: oyc_109

Also found by: 0x1f8b, 0x52, 0xNazgul, 0xSmartContract, 0xf15ers, 8olidity, Aymen0909, Bnke0x0, CertoraInc, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, IllIllI, JC, Lambda, Noah3o6, NoamYakov, RedOneN, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, TomJ, Twpony, Waze, Yiko, __141345__, ajtra, apostle0x01, ashiq0x01, asutorufos, bardamu, benbaessler, berndartmueller, bharg4v, bulej93, c3phas, cccz, ch13fd357r0y3r, codexploder, cryptonue, cryptphi, defsec, djxploit, durianSausage, fatherOfBlocks, gogo, hansfriese, horsefacts, ignacio, kyteg, lucacez, mics, rbserver, robee, sashik_eth, simon135, sseefried, tofunmi, xiaoming90

Awards

56.3243 USDC - $56.32

Labels

bug

QA (Quality Assurance)

External Links

Link to GitHub issue

no check on address (0)

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L53-L54 https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/ReceiverImplementation.sol#L86 https://github.com/code-423n4/2022-07-axelar/blob/a1205d2ba78e0db583d136f8563e8097860a110f/xc20/contracts/XC20Wrapper.sol#L27

dont use transfer instead use call

 receiver.transfer(amount);

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L128

tranfser of a token can be void will revert because there so no returndata of bool/memory bytes comming back

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L12

transfer of a token that will return false but succeded will revert to

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L12

salt shoudnt be set by user or contract is should be statevaraible that should change everytime its called because its random and if an attacker knows the salt because the blockchain is public the attacker can rellay the tx.

   function addressForTokenDeposit(
        bytes32 salt,
        address refundAddress,
        string calldata destinationChain,
        string calldata destinationAddress,
        string calldata tokenSymbol
    ) external view returns (address) {
        return
            _depositAddress(
                salt,
                abi.encodeWithSelector(
                    ReceiverImplementation.receiveAndSendToken.selector,
                    refundAddress,
                    destinationChain,
                    destinationAddress,
                    tokenSymbol
                )
            );
    }

https://github.com/code-423n4/2022-07-axelar/blob/a46fa61e73dd0f3469c0263bc6818e682d62fb5f/contracts/deposit-service/AxelarDepositService.sol#L44

there is no modifer to make sure msg.sender = AxelarDepositService, so users can drain eth from the contract

	  ) external {
        // Always refunding native otherwise it's sent on DepositReceiver self destruction
        if (address(this).balance > 0) refundAddress.transfer(address(this).balance);

make sure there is modier to make sure that only AxelarDepositReciver can call it.

#0 - re1ro
2022-08-05T09:38:30Z

1 2

Dup #3 #4

3

Invalid. This is a classic _safeTransferFrom implementation. success == true if it didn't revert and that's enough to pass the if statement

4

If token transfer returns false it's considered as failed

5

There is no possibility of replaying anything. Salt is used to generate distinctive addresses for the same user/intent. If you replay somebody's address you will sent your money to their destination.

6

Invalid. DepositReceiver is deployed and immediately destroyed. It has only constructor, there is nowhere users can re-enter. DepositReceiver can be deployed only by AxelarDepositService to have the right address

#1 - GalloDaSballo
2022-09-04T21:08:22Z

## no check on address (0) L

dont use transfer instead use call

3

Disputed https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/gas-service/AxelarGasService.sol#L164

transfer of a token that will return false but succeded will revert to

That's not a standard ERC20 as explicitly returning false means something went wrong

salt shoudnt be set by user or contract is should be statevaraible that should change everytime its called because its random and if an attacker knows the salt because the blockchain is public the attacker can rellay the tx.

Disagree

there is no modifer to make sure msg.sender = AxelarDepositService, so users can drain eth from the contract

Disputed per sponsor reply

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0xNazgul, 0xsam, 8olidity, Aymen0909, Bnke0x0, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, Fitraldys, JC, Lambda, MiloTruck, Noah3o6, NoamYakov, RedOneN, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, TomJ, Tomio, Waze, __141345__, a12jmx, ajtra, ak1, apostle0x01, asutorufos, benbaessler, bharg4v, bulej93, c3phas, defsec, djxploit, durianSausage, erictee, fatherOfBlocks, gerdusx, gogo, kyteg, lucacez, medikko, mics, owenthurm, oyc_109, rbserver, robee, sashik_eth, simon135, tofunmi

Awards

31.8812 USDC - $31.88

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

get the keccak hash ofline and put that value to constant to save gas

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L30-L44

when getting the keccak hash of value you should make the variables immutable to save gas

https://github.com/code-423n4/2022-07-axelar/blob/3373c48a71c07cfce856b53afc02ef4fc2357f8c/contracts/AxelarGateway.sol#L30-L44

onlyowner functions can be made payable because payable there is no check if msg.value ==0

https://github.com/code-423n4/2022-07-axelar/blob/a1205d2ba78e0db583d136f8563e8097860a110f/xc20/contracts/XC20Wrapper.sol#L44-L46 https://github.com/code-423n4/2022-07-axelar/blob/a1205d2ba78e0db583d136f8563e8097860a110f/xc20/contracts/XC20Wrapper.sol#L44-L73

make i++ into ++i to save 3 gas on a copy

AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {
deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
gas-service/AxelarGasService.sol:123:        for (uint256 i; i < tokens.length; i++) {

cache array.lenght to a memory variable

	AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {
deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
gas-service/AxelarGasService.sol:123:        for (uint256 i; i < tokens.length; i++) {

make i unchecked when it wont overflow

you are saving gas on solidity not checking on an overflow

	AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {
deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
gas-service/AxelarGasService.sol:123:        for (uint256 i; i < tokens.length; i++) {

#0 - re1ro
2022-08-05T09:21:36Z

1

Compiler does that

2

Dup #12

3

Dup #7 (5)

4 - 6

Dup #2

#1 - GalloDaSballo
2022-08-23T01:11:33Z

when getting the keccak hash of value you should make the variables immutable to save gas

Not true for ages (maybe like 4 years) https://twitter.com/GalloDaSballo/status/1543729080926871557

Rest will save less than 300 gas

Axelar Network v2 contest - simon135's results

Decentralized interoperability network.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-57] QA Report - $56.32

Findings Information

Awards

Labels

External Links

no check on address (0)

dont use transfer instead use call

tranfser of a token can be void will revert because there so no returndata of bool/memory bytes comming back

transfer of a token that will return false but succeded will revert to

salt shoudnt be set by user or contract is should be statevaraible that should change everytime its called because its random and if an attacker knows the salt because the blockchain is public the attacker can rellay the tx.

there is no modifer to make sure msg.sender = AxelarDepositService, so users can drain eth from the contract

#0 - re1ro2022-08-05T09:38:30Z

1 2

3

4

5

6

#1 - GalloDaSballo2022-09-04T21:08:22Z

dont use transfer instead use call

3

transfer of a token that will return false but succeded will revert to

salt shoudnt be set by user or contract is should be statevaraible that should change everytime its called because its random and if an attacker knows the salt because the blockchain is public the attacker can rellay the tx.

there is no modifer to make sure msg.sender = AxelarDepositService, so users can drain eth from the contract

[G-51] Gas Report - $31.88

Findings Information

Awards

Labels

External Links

get the keccak hash ofline and put that value to constant to save gas

when getting the keccak hash of value you should make the variables immutable to save gas

onlyowner functions can be made payable because payable there is no check if msg.value ==0

make i++ into ++i to save 3 gas on a copy

cache array.lenght to a memory variable

make i unchecked when it wont overflow

#0 - re1ro2022-08-05T09:21:36Z

1

2

3

4 - 6

#1 - GalloDaSballo2022-08-23T01:11:33Z

when getting the keccak hash of value you should make the variables immutable to save gas

#0 - re1ro
2022-08-05T09:38:30Z

#1 - GalloDaSballo
2022-09-04T21:08:22Z

#0 - re1ro
2022-08-05T09:21:36Z

#1 - GalloDaSballo
2022-08-23T01:11:33Z