0xmovses
commented
1 month ago In the origin Initiator contract the function function completeBridgeTransfer(bytes32 bridgeTransferId, bytes32 preImage) external is intended to be callable by anyone not only the bridge operator, not of course that it requires a preImage arg, so it is implicitly only callable by a user that posses the secret or preImage, therefore, it should not be restricted to only the bridge operator, as the operator would not be the user initiating the transfer and so, would not posses the secret.
@l-monninger to comment on access controls relating to setGenesisCeremony function.
l-monninger
(HAL-26) LACK OF ACCESS CONTROL ISSUES // CRITICAL
Auditor: Halborn Severity: Critical
Description Several access control issues have been identified in the movement codebase, distributed between the MCR staking contract, and the bridge contracts.
Lack Of Access Control In Staking Contract Functions The MovementStaking.sol contract contains critical vulnerabilities due to a lack of proper access control on several key functions. This allows any user to manipulate the staking system, leading to theft of funds, unfair distribution of rewards, and loss of trust in the protocol.
Affected functions are:
SET STAKING CEREMONY FUNCTION Unrestricted access to the staking setStakingCeremony function allows any attester to stake a small amount and steal the total stake on the contract by setting themselves as the sole attester with an inflated stake amount.
Any attester can call this function and set the genesis ceremony state, including defining which attesters are included and their stake amounts. The function does not properly validate that all existing attesters are included or that their stakes are accurately represented:
This vulnerability allows attackers to manipulate the stakes of other attesters, either stealing the stakes, or leading to unfair repartition of the rewards. Such an attack would also cause a loss of trust and reputation toward the protocol.
REWARD AND STAKING FUNCTIONS A lack of access control on the reward and slash functions of the MovementStaking.sol contract allows anyone to arbitrarily reward or slash attesters, enabling an attacker to drain funds from the contract or manipulate the staking system.
By setting his address in the reward function, the attacker can simply specify how much to receive from the contract:
The vulnerable reward function:
Bridge Service Access Control Issues The bridge contract do not enforce access control verifying that the caller is the bridge service. These issues happen in:
AtomicBridgeInitiator.sol AtomicBridgeCounterparty.sol BRIDGE INITIATOR COMPLETE TRANSFER IS NOT RESTRICTED The completeBridgeTransfer of the EVM initiator contract is not restricted to only its owner.
In a normal sequence, the user is supposed to call completeBridgeTransfer on the counterparty contract, which will emit an event that will be relayed on the initiator contract by the bridge service. The service will forward the event to completeBridgeTransfer on the initiator contract to update the status of the bridge transfer to COMPLETED.
As the function does not have access control verifying that the bridge service is the caller, any user can therefore call the function, updating the state to COMPLETED and provoke unplanned errors the bridge service trying to call the function in the future, reverting:
The bridge service is not implemented yet and therefore it is not possible to assess the real impact of the finding in term of denial of service or logic bugs precisely.
NO ACCESS CONTROL ON COUNTERPARTY ABORT TRANSFER FUNCTION The AtomicBridgeCounterparty.sol contract lacks of access control on the abortBridgeTransfer function: it should verify that the bridge service is the caller.
The normal sequence of the transaction refund is:
User calls the refundBridgeTransfer function on the initiator. The bridge relays the refund transfer event to the counterparty through the abortBridgeTransfer function, to update the transfer state to REFUNDED.
If a user calls abortBridgeTransfer himself, the transfer state will change to REFUNDED and the bridge service attempt will revert because the function makes sure that the state is beginning as INITIALIZED.
That can cause denial of service or logical bugs in the bridge, depending on the implementation. As the implementation was not completed during the assessment, it was not possible to precisely assess the impact of such a vulnerability.
Proof of Concept The most critical issues are localized in the staking contract, and the following PoCs highlight these vulnerabilities:
SET GENESIS CEREMONY FUNCTION In this PoC, Bob is able to call setGenesisCeremony and set himself as the only attester with a stake of 1000 tokens, despite only having staked 100 tokens initially. This results in Bob receiving a refund of 900 tokens, effectively stealing Alice's stake.
REWARD AND STAKING FUNCTIONS This PoC shows that any user (Charlie in this case) can call the reward function to arbitrarily allocate rewards to themselves or others, effectively stealing funds from the staking contract.
The result of the test:
BVSS AO:A/AC:L/AX:L/R:N/S:C/C:N/A:N/I:N/D:C/Y:C (10.0)
Recommendation For the staking contract, it is recommended to:
For the bridge contracts, it is recommended to: