It is possible for an attacker to accept their commitment without supermajority due to the [race condition]

[SA124]

Auditor: Ottersec Code: MCR Settlement Contract

1. It is possible for an attacker to accept their commitment without supermajority due to the race condition in submitBlockCommitmentForAttester. Consider a scenario where attacker has 100 stake and other users have 100 as well and current epoch is 10. Attacker calls submitBlockCommitmentForAttester and the block gets assigned epoch. Attacker then unstakes all of their stake for epoch 11. Once the current epoch switches to 11, the computeAllTotalStakeForEpoch(11) is now equal to 100, since attacker’s funds are unstaked. But attacker-submitted commitment still has value of 100 in commitmentStakes, which is supermajority for the current total stake, which means contract will accept the attacker’s commitment without them having supermajority vote in reality.

~2. Instead of msg.sender domain, attesters[i] is passed to getCurrentEpoch in slash. This could lead to the refund amount miscalculation due to wrong epoch and subsequently loss of funds.~ #498

~3. It’s possible for current epoch to go beyond yet uncommited block’s epoch assignments: Let’s assume there’s two commitments in queue: A(H=5, blockHeightEpochAssignments=10) and B(H=6, blockHeightEpochAssignments=10). The time based rollover in _acceptBlockCommitment here might roll over to the 11th epoch when accepting the first block, causing a Denial of Service for the contract since the epoch check here can never pass once this happens.~ #499

~4. Malicious attester might be able to fully control accepted blockId with small stake. Let’s assume Commitment(H, C, BID) is expected to get the supermajority soon. Malicious attester can submit a commitment: Commitment(H, C, BID_CORRUTED). Since commitment is the same, all the former votes will technically also count towards the attacker’s submission. If attacker gets lucky and is stored on the first index of attesters array, their commitment with corrupted blockId will be accepted and used (code here).~ #448

Nits/questions:

• In stake, _addStake is called with getNextEpochByBlockTime but AttesterStaked event records getNextEpoch which could be different. Same for unstake.

• Could use getMaxTolerableBlockHeight here to make code clearer

[mzabaluev]

The submission has been copied from a list on Slack and lists several unrelated issues.

5. Malicious attester might be able to fully control accepted blockId with small stake. Let’s assume Commitment(H, C, BID) is expected to get the supermajority soon. Malicious attester can submit a commitment: Commitment(H, C, BID_CORRUTED). Since commitment is the same, all the former votes will technically also count towards the attacker’s submission. If attacker gets lucky and is stored on the first index of attesters array, their commitment with corrupted blockId will be accepted and used (code here).

I believe this is the same as #448.

[l-monninger]

Marking as decentralized because MCR does not need to be safe for centralized launch. See #499.