code-423n4 / 2023-09-ondo-findings

7 stars 5 forks source link

Bridged messages can still be approved and executed even if contract is paused #443

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L337

Vulnerability details

The destination bridge contract can be paused to prevent execution of bridged messages and token minting. However, pending transactions can still be approved and its associated tokens minted while the contract is paused.

Impact

Pauses in the DestinationBridge contract affect the _execute() function, which is the implementation that gets called when Axelar relays a message to the destination contract:

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L85-L89

85:   function _execute(
86:     string calldata srcChain,
87:     string calldata srcAddr,
88:     bytes calldata payload
89:   ) internal override whenNotPaused {

However, transactions are also subject to an approval phase before token minting happens. After a bridged message is received, the transaction is stored to be reviewed and tokens are minted only after the required number of approvals are granted.

This means that already bridged messages can still lead to token minting even if the contract is under pause. Since the whenNotPaused modifier is only applied to the _execute() function, pending transactions can still be approved, allowing token minting while the contract is paused.

Proof of Concept

  1. DestinationBridge contract receives a message and stores the transaction to be reviewed.
  2. Admin pauses DestinationBridge contract.
  3. Transaction is approved and tokens are minted.

Recommended Mitigation Steps

Add the whenNotPaused modifier to the _mintIfThresholdMet() function. This will prevent token minting for any possible flow if the contract is paused.

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #189

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #529

c4-judge commented 1 year ago

kirk-baird marked the issue as unsatisfactory: Invalid

c4-judge commented 1 year ago

kirk-baird removed the grade

c4-judge commented 1 year ago

kirk-baird changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

kirk-baird marked the issue as grade-a