Assessment reports>Initia>High findings>Published module names do not necessarily match binary module
Category: Coding Mistakes

Published module names do not necessarily match binary module

High Severity
Informational Impact
N/A Likelihood

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.

Zellic © 2024Back to top ↑