code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

QA Report #232

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Table of Contents: L-01 Unused receive() function will lock Ether in contract L-02 Missing checks for approve()’s return status L-03 Unsafe casts and usage of IERC20 L-04 Open TODOs L-05: abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

N-01 Lock pragmas to specific compiler version N-02 Use a more recent version of solidity N-03 Files are missing NatSpec N-04 Event is missing indexed fields N-05 public functions not called by the contract should be declared external instead N-06 Remove commented out code

L-01 Unused receive() function will lock Ether in contract If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

Instances include: DepositReceiver.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol#L29

AxelarDepositServiceProxy.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositServiceProxy.sol#L13

L-02 Missing checks for approve()’s return status Some tokens, such as Tether (USDT) return false rather than reverting if the approval fails. Use OpenZeppelin’s safeApprove(), which reverts if there’s a failure, instead.

Instances include: AxelarDepositService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L30

ReceiverImplementation.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L38 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L64

L-03 Unsafe casts and usage of IERC20 not all ERC20 contracts define decimals() since it’s optional in the spec. Use safeDecimals() instead.

Instances include: XC20Wrapper.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L62

L-04 Open TODOs Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.

Instances include: AxelarGateway.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L29 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L226 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L227 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L267

L-05: abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). “Unless there is a compelling reason, abi.encode should be preferred”. If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() https://ethereum.stackexchange.com/questions/30912/how-to-compare-strings-in-solidity#answer-82739

Instances include: AxelarGateway.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L298 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L342 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L540 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L544 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L548 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L552 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L556

AxelarDepositService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L228 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositService.sol#L233

DepositBase.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositBase.sol#L38

N-01 Lock pragmas to specific compiler version Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively. see https://swcregistry.io/docs/SWC-103

Instances include: IDepositBase.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IDepositBase.sol#L3

IAxelarAuth.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuth.sol#L3

IAxelarGasService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L3

IAxelarDepositService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarDepositService.sol#L3

IAxelarAuthWeighted.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuthWeighted.sol#L3

N-02 Use a more recent version of solidity Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(,) Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

Instances include: AxelarGateway.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L3

IDepositBase.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IDepositBase.sol#L3

AxelarGasService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasService.sol#L3

AxelarGasServiceProxy.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/gas-service/AxelarGasServiceProxy.sol#L3

IAxelarAuth.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuth.sol#L3

IAxelarGasService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L3

IAxelarDepositService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarDepositService.sol#L3

IAxelarAuthWeighted.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuthWeighted.sol#L3

DepositReceiver.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositReceiver.sol#L3

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

DepositBase.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/DepositBase.sol#L3

ReceiverImplementation.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/ReceiverImplementation.sol#L3

AxelarDepositServiceProxy.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/deposit-service/AxelarDepositServiceProxy.sol#L3

AxelarAuthWeighted.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L3

XC20Wrapper.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L3

N-03 Files are missing NatSpec Instances: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IDepositBase.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasServiceProxy.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarAuth.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarGasService.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarDepositService.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarAuthWeighted.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/DepositReceiver.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/DepositBase.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/ReceiverImplementation.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositServiceProxy.sol https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol

N-04 Event is missing indexed fields Each event should use three indexed fields if there are three or more fields

Instances include: IAxelarGasService.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L13-L21 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L23-L33 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L35-L42 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L44-L53 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L55 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarGasService.sol#L57

IAxelarAuthWeighted.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/interfaces/IAxelarAuthWeighted.sol#L14

N-05 public functions not called by the contract should be declared external instead Contracts are allowed to override their parents’ functions and change the visibility from external to public. https://docs.soliditylang.org/en/latest/contracts.html#function-overriding

Instances include: XC20Wrapper.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/xc20/contracts/XC20Wrapper.sol#L30

N-06 Remove commented out code

Instances include: AxelarGateway.sol https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L23 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L24

GalloDaSballo commented 2 years ago

L-01 Unused receive() function will lock Ether in contract

Valid for DepositServiceProxy L

 L-02 Missing checks for approve()’s return status

L

L-03 Unsafe casts and usage of IERC20

L

L-04 Open TODOs

NC

L-05: abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Disputed for those cases

N-01 Lock pragmas to specific compiler version

NC

N-02 Use a more recent version of solidity

NC

N-03 Files are missing NatSpec

NC

N-04 Event is missing indexed fields

Disagree

 N-05 public functions not called by the contract should be declared external instead

R

N-06 Remove commented out code

NC

3L 1R 5NC