ISM configuration of MailboxComponent is disregarded
Description
In the Hyperlane protocol, a recipient can specify the ISM to be used by having the public method interchainSecurityModule(). The Solidity implementation of MailboxClient contract allows a contract to specify the ISM through providing the getter/setter functions for ISM:
abstract contract MailboxClient is OwnableUpgradeable {
// ...
IInterchainSecurityModule public interchainSecurityModule;
// ...
function setInterchainSecurityModule(
address _module
) public onlyContractOrNull(_module) onlyOwner {
interchainSecurityModule = IInterchainSecurityModule(_module);
}
// ...
}The mailboxclient_component.cairo also has getter/setter functions for its ISM:
#[starknet::component]
pub mod MailboxclientComponent {
// ...
#[storage]
struct Storage {
// ...
interchain_security_module: ContractAddress,
}
// ...
#[embeddable_as(MailboxClientImpl)]
impl MailboxClient<
// ...
> of IMailboxClient<ComponentState<TContractState>> {
// ...
fn set_interchain_security_module(
ref self: ComponentState<TContractState>, _module: ContractAddress
) {
let ownable_comp = get_dep_component!(@self, Owner);
ownable_comp.assert_only_owner();
assert(_module != contract_address_const::<0>(), Errors::ADDRESS_CANNOT_BE_ZERO);
self.interchain_security_module.write(_module);
}
// ...
fn get_interchain_security_module(
self: @ComponentState<TContractState>
) -> ContractAddress {
self.interchain_security_module.read()
}
// ...
}
// ...
}It should be noted that the storage variable does not automatically create the getter method for the variable. In this contract, the function get_interchain_security_module() is the getter method.
However, the method the Mailbox utilizes to fetch the ISM of the recipient is the interchain_security_module(), not the get_interchain_security_module().
Impact
MailboxClient has getter/setter functions about ISM, which do not affect the logic of contract.
While this does not pose a direct security risk, this issue can still lead to confusion, potential misuse of the contracts, and inefficient code.
Recommendations
Consider renaming the method get_interchain_security_module to interchain_security_module in order to provide the expected functionality.
Remediation
This issue has been acknowledged by Pragma, and a fix was implemented in commit cc203103↗.