Published module names do not necessarily match binary module
Description
Initia supports publishing Move modules from Move code. It is implemented as a publish request, which is posted from the Move code, and is executed at the end of the transaction. The newly published module will not be available until the next transaction. There can only be one pending request per transaction, but it is possible to publish an arbitrary number of modules simultaneously at the same owner address.
The modules are taken as two parallel arrays of the same length, one for the names of the modules to be published and the other for the actual binary modules to be published. In the following code, the module names are used for checking that the upgrade policy of the module allows the action that is about to be taken, but the binary modules are the actually published modules.
public entry fun publish(
owner: &signer,
module_ids: vector<String>, // 0x1::coin
code: vector<vector<u8>>,
upgrade_policy: u8,
) acquires ModuleStore, MetadataStore {
// Disallow incompatible upgrade mode. Governance can decide later if this should be reconsidered.
assert!(vector::length(&code) == vector::length(&module_ids), error::invalid_argument(EINVALID_ARGUMENTS));
// [...]
// Check upgradability
let metadata_table = &mut borrow_global_mut<MetadataStore>(addr).metadata;
vector::for_each_ref(&module_ids,
|module_id| {
if (table::contains<String, ModuleMetadata>(metadata_table, *module_id)) {
let metadata = table::borrow_mut<String, ModuleMetadata>(metadata_table, *module_id);
assert!(metadata.upgrade_policy < UPGRADE_POLICY_IMMUTABLE,
error::invalid_argument(EUPGRADE_IMMUTABLE));
assert!(can_change_upgrade_policy_to(metadata.upgrade_policy, upgrade_policy),
error::invalid_argument(EUPGRADE_WEAKER_POLICY));
// [...]
Note that there is no check that the i-th element in module_ids
matches the name of the i-th element in code
.
Impact
On the surface, it looks like it is possible to publish a module while using and updating the upgrade policy of another module, allowing for a bypass of the module-upgrade restrictions. However, due to a coincidence of the structure of the code, the impact is completely mitigated.
The passed-in module names are first put into a set in the following code.
let mut expected_modules: BTreeSet<String> = BTreeSet::new();
for name in safely_pop_vec_arg!(arguments, Struct) {
let str_bytes = get_string(name)?;
context.charge(gas_params.per_byte * NumBytes::new(str_bytes.len() as u64))?;
expected_modules.insert(String::from_utf8(str_bytes).map_err(|_| {
SafeNativeError::Abort {
abort_code: EUNABLE_TO_PARSE_STRING,
}
})?);
}
Right before publishing, the following check is performed to ensure that all published modules have been named in the name list.
if let Some(mut expected_modules) = expected_modules {
for m in modules {
if !expected_modules.remove(m.self_id().short_str_lossless().as_str()) {
return Err(metadata_validation_error(&format!(
"unregistered module: '{}'",
m.self_id().name()
)));
}
}
}
These two checks ensure that while the wrong permissions may be checked for a given module, any published module must be named in the list of modules names. This means any published module's permissions must be checked anyway, even if it is in the incorrect slot in the names array.
Recommendations
Ensure that the names of the modules match the actual name of the binary module.
Remediation
This issue has been acknowledged by Initia Labs, and a fix was implemented in commit ae04bf3f↗.