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

0 stars 0 forks source link

QA Report #88

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

QA Report

Non-Critical Findings

[NC-01] DepositBase.sol imports IERC20 and IWETH9 but never uses them

Description

Waste of gas to import interfaces and then never use them

Lines of Code

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

Mitigation

Remove imports in L6 and L7

[NC-02] IWETH9.sol should add approve function

Description

When calling approve for WETH contracts cast WETH address as an IERC20 and call approve through that. For better readability, IWETH9 should have an approve function and that should be used instead of recasting as IERC20 and using that approve

Lines of Code

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

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

Mitigation

IWETH9.sol should add approve function and it should be used instead of IERC20 when calling approve for WETH

Low Risk Findings

[L-01] Use of floating pragma

Description

Contracts should be deployed with the same compiler version and flags that they have been designed and tested. Different compiler versions/flags may introduce bugs that affect the contract negatively. Locking the pragma helps avoid this.

Lines of Code

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

Mitigation

Match the rest of the code base and lock to 0.8.9:

pragma solidity 0.8.9; 

[L-02] XC20Wrapper.sol#removeWrapping doesn't reset metadata

Description

XC20Wrapper.sol#removeWrapping doesn't remove the metadata from the XC20 token responsible for wrapping the axelar token. This could be confusing to users because metadata will specify a token that can't actually be wrapped/unwrapped. This could lead to users trying to wrap tokens and wasting gas.

Lines of Code

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

Mitigation

Add line to reset metadata when calling XC20Wrapper.sol#removeWrapping

[L-03] XC20Wrapper.sol doesn't implement _execute

Description

AxelarExecutable.sol is an abstract that doesn't implement _execute. XC20Wrapper.sol inherits from AxelarExecutable.sol but never implements _execute

Lines of Code

N/A

Mitigation

Implement XC20Wrapper.sol#_execute

[L-04] AxelarGatewayProxy.sol#fallback should not be payable

Description

Documentation makes it clear that AxelarGateway.sol should never receive ETH, therefore fallback should not be payable.

Lines of Code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGatewayProxy.sol#L28

Mitigation

Remove payable from AxelarGatewayProxy.sol#fallback

re1ro commented 2 years ago

NC1

Yup. Dup #52

NC2

Ack

L1

Dup #8

L2

Ack

L3

This is intentional. We only expect only crosschain calls with token attached.

L4

Good spot

GalloDaSballo commented 2 years ago

[NC-01] DepositBase.sol imports IERC20 and IWETH9 but never uses them

NC

[NC-02] IWETH9.sol should add approve function

NC

[L-01] Use of floating pragma

NC

[L-02] XC20Wrapper.sol#removeWrapping doesn't reset metadata

Seems informational, R

[L-03] XC20Wrapper.sol doesn't implement _execute

Valid Low although the sponsor claims no impact

[L-04] AxelarGatewayProxy.sol#fallback should not be payable

Valid Low

2L 1R 3NC