Omniscia Etherlink Audit
WXTZToken Manual Review Findings
WXTZToken Manual Review Findings
WXT-01M: Incorrect Relay of Source ID
Type | Severity | Location |
---|---|---|
Logical Fault | WXTZToken.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:
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
Type | Severity | Location |
---|---|---|
Logical Fault | WXTZToken.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:
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
Type | Severity | Location |
---|---|---|
Language Specific | WXTZToken.sol:L15 |
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:
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.