Omniscia Etherlink Audit

WXTZToken Manual Review Findings

WXTZToken Manual Review Findings

WXT-01M: Incorrect Relay of Source ID

TypeSeverityLocation
Logical FaultWXTZToken.sol:L83, L88

Description:

The WXTZToken::_credit function override will not properly relay the source chain ID to the internal OFT::_credit function.

Impact:

While the OFT::_credit function implementation the code inherits from ignores the relevant argument, it is best practice to relay it in case of an unforeseen dependency update.

Example:

contracts/WXTZToken.sol
71emit Withdrawal(msg.sender, wad);
72}
73
74/**
75 * @dev Override the _credit method to add protection in case the multisignature owner of the Oapp gets hacked,
76 * and people can use the setPeer method to setPeer a malicious OFT on another chain.
77 * Only the bridged OFTs are at risk.
78 * @param _to The address to credit the tokens to.
79 * @param _amountLD The amount of tokens to credit in local decimals.
80 * @dev _srcEid The source chain ID.
81 * @return amountReceivedLD The amount of tokens ACTUALLY received in local decimals.
82 */
83function _credit(
84 address _to,
85 uint256 _amountLD,
86 uint32 /*_srcEid*/
87) internal override returns (uint256 amountReceivedLD) {
88 if (block.chainid == 128123 || block.chainid == 42793) {
89 require(_amountLD + totalSupply() <= address(this).balance);

Recommendation:

We advise it to be relayed properly by uncommenting the relevant variable and supplying it in place of the 0 value literal in the super._credit call.

Alleviation:

The _srcEid member is properly relayed to the superlative OFT::_credit call per our recommendation, alleviating this exhibit.

WXT-02M: Inexplicable Simultaneous Support of Testnet & Mainnet

TypeSeverityLocation
Logical FaultWXTZToken.sol:L26, L85

Description:

The WXTZToken implementation is compatible with both the testnet and main-net deployments which we consider bad practice as a compromise of the testnet deployment might cause the token to be linked with the main-net deployment.

Impact:

A potential compromise of an otherwise simple environment (Etherlink Testnet) might have devastating consequences for the main-net WXTZToken implementation due to the code's incorrect multi-chain support.

Example:

contracts/WXTZToken.sol
22/**
23 * @dev This modifier can be applied to methods that should only be callable on Etherlink testnet or mainnet.
24 */
25modifier onlyEtherlink() {
26 require(block.chainid == 128123 || block.chainid == 42793);
27 _;
28}

Recommendation:

We advise the token to accept the main-net chain ID as an input argument during its WXTZToken::constructor, and its value to be assigned to an immutable contract level variable thereby not affecting the contract's gas costs.

Alleviation:

The code was updated per our recommendation, implementing an immutable chain ID to signify the Etherlink chain and thus addressing the duality outlined by this exhibit.

WXT-03M: Potential Cross-Chain Security Enhancement

Description:

The WXTZToken represents a highly sensitive contract implementation meant to wrap the native token of the Etherlink network. As such, maximal security should be preferred when handling it to avoid potential cross-chain compromises from affecting it.

Impact:

We believe that the current security status of the token can be enhanced when it comes to its main-net deployment as it will become a crucial component underpinning the Etherlink network.

Example:

contracts/WXTZToken.sol
71emit Withdrawal(msg.sender, wad);
72}
73
74/**
75 * @dev Override the _credit method to add protection in case the multisignature owner of the Oapp gets hacked,
76 * and people can use the setPeer method to setPeer a malicious OFT on another chain.
77 * Only the bridged OFTs are at risk.
78 * @param _to The address to credit the tokens to.
79 * @param _amountLD The amount of tokens to credit in local decimals.
80 * @dev _srcEid The source chain ID.
81 * @return amountReceivedLD The amount of tokens ACTUALLY received in local decimals.
82 */
83function _credit(
84 address _to,
85 uint256 _amountLD,
86 uint32 /*_srcEid*/
87) internal override returns (uint256 amountReceivedLD) {
88 if (block.chainid == 128123 || block.chainid == 42793) {
89 require(_amountLD + totalSupply() <= address(this).balance);

Recommendation:

We advise the token's WXTZToken::setPeer mechanism to be overridden, and different logic to be executed when the Etherlink network is detected. Specifically, we advise a configured peer to be irreplaceable and new peer configurations to take effect after a timelock, preventing immediate compromises of the WXTZToken and allowing adequate time for WXTZ token holders to migrate back and unwrap if a malicious WXTZToken::setPeer proposal is detected.

Alleviation:

The Etherlink team evaluated this exhibit, and opted to apply a timelock to peer adjustments that adhere to a 2-day window permitting users to react properly to potentially unwanted cross-chain configurations.

While the first of our two recommendations was not heeded, we extensively discussed with the Etherlink team and concluded that it would not align with their business requirements and overall ethos.

As such, we consider this exhibit alleviated to the greatest extent possible in alignment with the team's intentions.