hats-finance / Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d

Aleph Zero bridge to Ethereum
Apache License 2.0
0 stars 1 forks source link

`upgrade()` functionality is missing for `ink` based contracts migration #40

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x5aaf50f06fb781828bd17c6096ac70da27ee8bc706c0e0981fe9d7ee459d95fb Severity: medium

Description: Description\

Affected code:

https://github.com/Cardinal-Cryptography/most/blob/a67f7a523f4257415ca74ba8d9a27dde3b313193/azero/contracts/migrations/lib.rs#L1-L68

In Most(Ink contracts), Migration contract is designed to facilitate a series of upgrades or migrations on Aleph Zero. It serves as a central registry to track the progress of these migrations and ensure a controlled, authorized upgrade process.

Before understanding the issue, Let's understand how upgrade process works:

1) The Migration contract owner deploys a new version (upgrade) of the contract.

2) The owner calls the upgrade() function on the existing Migrations contract, providing the address of the newly deployed upgrade contract.

3) The upgrade function verifies that the provided address points to a valid Migrations contract.

4) It then creates an instance of the upgraded contract and calls its setCompleted() function with the current last_completed_migration value. This ensures that the migration history is preserved in the upgraded contract.

5) In essence, this migration contract provides a secure and controlled way to manage upgrades within a dApp on the Aleph Azero.

The heart of Whole migration process is upgrade() function which is missing in contracts which can be checked here.

Therefore, migration to new address is not possible due to missing upgrade functionality in Migration contract.

It should be noted that, in solidity Migration contract, upgrade() function was provided to carryout the upgrade process and it is shown as:


    function upgrade(address new_address) public restricted {
        Migrations upgraded = Migrations(new_address);
        upgraded.setCompleted(last_completed_migration);
    }

POC: 1) The owner upgrades the contract and now looks into migration. 2) The owner tries to call upgrade() function to pass the new address so that migration can be successfully done and the new address point to valid migration contract but he can not do it as there is no upgrade() function missing to do this on Aleph Azero. 3) This makes the Migration contract more or useless as the upgradation can not be done via new address.

Such upgrade() function is provided in solidity contracts but missed in ink contracts.

The severity of this issue is identified as Medium severity as this breaks the intended core design functionality of ink contracts related to migrations.

Recommended Mitigation steps\

Add upgrade() function in Migration ink based contract here.

Note: Solidity based Migration.sol can be referred here.

fbielejec commented 4 months ago

Invalid, out-of-scope submission. Migrations contracts on both chains are just a counter, opt-in and used in migration scripts to ensure they are applied additively. They will never need to be updated, as a new contract can always be deployed. If anything the upgrade tx should be removed from the Solidity version of the contract as unnecessary.