Chromia - Postchain

In `withdrawBySnapshot()`, several merkle proofs are provided to validate the `snapshot`. The merkle root containing all of the snapshots is in `extraProof.leaf`. The `stateProof` is a merkle proof that validates that the `snapshot` is in the `extraProof.leaf`. However, there is no validation on `extraProof.root` to confirm that it is part of the `massExitBlock.blockRid`. While `TokenBridge._withdrawRequest()` calls `verifyBlockHeader()`, this step is skipped in `withdrawBySnapshot()`. If any `extraProof.root` can be forged, this would allow anyone to drain the contract during a mass exit. The relevant code has been reproduced below: ```js function withdrawBySnapshot( bytes calldata snapshot, Data.Proof memory stateProof, Data.ExtraProofData memory extraProof ) public virtual whenMassExit whenNotPaused nonReentrant { require(_snapshots[stateProof.leaf] == false, "TokenBridge: snapshot already used"); require(stateProof.leaf == keccak256(snapshot), "TokenBridge: snapshot data is not correct"); require(Hash.hashGtvBytes64Leaf(extraProof.leaf) == extraProof.hashedLeaf, "Postchain: invalid EIF extra data"); // Internal integrity of the `extraProof` is validated. bytes32 stateRoot = _bytesToBytes32(extraProof.leaf, 32); // The final reference to `extraProof` if (!MerkleProof.verify(stateProof.merkleProofs, stateProof.leaf, stateProof.position, stateRoot)) // The final reference to `stateRoot`. No validation against the `massExitBlock`. revert("TokenBridge: invalid merkle proof"); .... } ```

`_withdrawRequest()` allows withdrawals during a mass exit so long as the block is older than the `massExitBlock`. This allows users to execute withdrawals that they've already initiated on the Chromia side, as they would not appear in the token snapshot for the `massExitBlock`. Though the old block will predate the `massExitBlock`, signifying that it was mined before the validators were compromised, it is still the compromised validators who are signing off on the submitted `blockRid` in the withdrawal request. The compromised validators can approve a forged `blockRid` that claims to be older than `massExitBlock`.

In the operation `link_evm_eoa_account()`, two authentications occur: 1. `authenticate()` is called to get the account associated with the operation call. 1. `ft4.auth.verify_signers([address]);` ensures that the EVM address has signed the message permitting the link. <br> However, the message signed by the EVM address does not appear to have any reference to the FT4 account. An example message format is as follows: ``` Blockchain: 0000000000000000000000000000000000000000000000000000000000000000 Please sign the message to call operation: - eif.hbridge.link_evm_eoa_account with arguments: - E1FAE9B4FAB2F5726677ECFA912D96B0B683E6A9 Nonce: 165436D801A7942A5D454666B91070AAFF5FE724CCEB74BA99830D380E17D399 ``` <br> In the above, `E1FAE9B4FAB2F5726677ECFA912D96B0B683E6A9` corresponds to the EVM address. The `Nonce` field does not contain any reference to the FT4 address except for the `auth_descriptor.ctr` (as seen in `utils.derive_nonce(op, auth_descriptor.ctr)` in `authentication.rell`). As long as the attacker has an `auth_descriptor` with the same `ctr` as the honest owner of the EVM address, they may be able to front-run linking the EVM address, potentially stealing deposited funds.

The bridge owner has the power to both call `triggerMassExit()` and `postponeMassExit()`. Despite its name, `postponeMassExit()` does not delay the mass exit, but instead returns the contract to normal operations. Additionally, conventional bridge withdrawals and withdrawals by snapshot are tracked differently. Completed withdrawals done by `withdrawRequest()` are tracked in the `_events` mapping and the withdrawals done by `withdrawBySnapshot()` are tracked in the `_snapshots` mapping. The token bridge owner can batch their transactions such that they trigger a mass exit, withdraw tokens by snapshot, and postpone the mass exit to return the contract to normal operations. They could then withdraw the same tokens normally at their convenience with the usual withdrawal method.

The token bridge owner can trigger a mass exit if the validators are suspected of being compromised. This allows all users to withdraw their tokens according to a snapshot at the last valid block. The owner is allowed to withdraw the remaining contract balance after 90 days. Additionally, the ability to pause and unpause the contract is spread between the contract owner and the validators. The validators can pause the contract and the owner can unpause it. Should the contract be paused for any reason, a malicious owner is incentivized to keep all withdrawals halted and guarantee the full contract balance for themselves. They can call `triggerMassExit()`, regardless of whether the validators have been compromised, and withdraw the full contract balance after 90 days.

A call to `postponeMassExit()` returns the contract back to normal functionality after a mass exit has been triggered. If any withdrawals have occurred, this would lead to a deficit in the bridge, as the contract will not have enough balance to honor all withdrawals made with `withdrawRequest()`. Given that this possibility has been mentioned in the documentation, the issue has been assigned Informational severity.

Function `ManagedValidator.setBlockchainRid()` lacks any access controls and may be called by anyone. They can set an arbitrary `blockchainRid` value, impacting any validations being performed on that contract.

The `transferMintRole()` and `transferOwnership()` functions use a two-week delay. The delay is reset if a new pending address is called. The relevant code is reproduced below: ```js function transferMintRole(address newMinter) external onlyOwner { if (pendingNewMinter != address(0)) resetDelayForFunction(this.transferMintRole.selector); startDelayedAction(this.transferMintRole.selector); pendingNewMinter = newMinter; } function transferOwnership(address newOwner) public override onlyOwner { if (pendingOwner() != address(0)) resetDelayForFunction(this.transferOwnership.selector); startDelayedAction(this.transferOwnership.selector); super.transferOwnership(newOwner); } ``` <br> `startDelayAction()` will revert if a delay is active. This means that these functions will always revert if the pending address is `address(0)` and there is an active delay. This could result in a cumbersome transfer of addresses as the owner waits for the two week delay to complete before reassigning the addresses. If a transfer of the minter role is needed, it could also lead to the `ChromiaToken_Base()` not having an assigned minter for a period of time. Depending on the requirements of the contract, having no minter assigned may be prohibitive, forcing a launch of a new contract or keeping the current minter.

The `TokenMinter` contract enforces a two-week delay for increasing the `dayLimit` and no delay for decreasing it. If a new decreased `dayLimit` is assigned, it will not clear any `pendingDayLimit` increases. This can result in the `dayLimit` being decreased, quickly followed by the previous `pendingDayLimit` completing its two-week delay and being assigned. The code has been reproduced below: <br> ```js function setDayLimit(uint _newDayLimit) external onlyOwner { if (_newDayLimit > dayLimit) { // if we already have a pending change, reset it if (pendingDayLimit != 0) resetDelayForFunction(this.setDayLimit.selector); // Set pending day limit and start the two-week delay pendingDayLimit = _newDayLimit; startDelayedAction(this.setDayLimit.selector); } else { // If the new limit is lower, apply immediately dayLimit = _newDayLimit; emit DayLimitChanged(_newDayLimit); } } function finishSetDayLimit() external onlyOwner { finishDelayedAction(this.setDayLimit.selector); dayLimit = pendingDayLimit; delete pendingDayLimit; emit DayLimitChanged(dayLimit); } ```

To initially configure the bridge, the owner can call `setBlockchainRid()` to point to the chain from which the bridge can receive withdrawals. However, the owner can reassign this value at any time, making it impossible to withdraw previous deposits.