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

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

Different variable type used between `Migrations.sol::last_completed_migration` and `migrations/lib.rs::last_completed_migration`. #29

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

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

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0xd15037392d9aa990c25375ccfc83fc724ed873b4b7722dadd8e6f214cd4a0afc Severity: minor

Description: Description

Different variable type used between Migrations.sol::last_completed_migration and migrations/lib.rs::last_completed_migration.

Attack Scenario

Ink language does not have a built-in u256 type. Instead, it provides u8, u16, u32, u64, and u128 types for unsigned integers of different sizes, up to 128 bits.

migrations/lib.rs::last_completed_migration uses u32 variable type:

#[ink(storage)]
    pub struct Migrations {
        last_completed_migration: u32,
        owner: AccountId,
    }

However in Migrations.sol::last_completed_migration, it uses uint256:

contract Migrations {
    address public immutable owner;
    uint256 public last_completed_migration;

//REDACTED BY ERICTEE

This discrepancy allow owners to set last_completed_migration higher than type(uint32).max in Migrations.sol::setCompleted but not in migrations/lib.rs::set_completed.

Attachments

NA

  1. Proof of Concept (PoC) File

Manual Analysis

  1. Revised Code File (Optional)

Consider changing the variable type in Migrations.sol::last_completed_migration to uint32 as well for consistency.

krzysztofziobro commented 4 months ago

Proposed change does not provide any improvement in functionality