BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: medium

The checking for `isTerminal` of outcome is not performed

Author Revealed upon completion

Root + Impact

The checking for PoolStates::isTerminal of outcome is not performed

The checking for the terminal state is only done through IAttackRegistry.ContractState

checking with outcome is not performed

so the checking for terminal state is inaccurate

Description

On PoolStates library we can see it has a function to check if the outcome is in terminal ot not

library PoolStates {
enum Outcome {
UNRESOLVED,
SURVIVED,
CORRUPTED,
EXPIRED
}
​
/// @notice Returns true when an outcome is terminal.
function isTerminal(Outcome outcome) internal pure returns (bool) { // <--@ it is not checked
return outcome != Outcome.UNRESOLVED;
}
}

But it is never used

Like on _observePoolState, it is checking for if the state is terminal ot not through IAttackRegistry.ContractState

and then calling for _markRiskWindowEnd

function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (
!scopeLocked && state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
) {
scopeLocked = true;
emit ScopeLocked(block.timestamp);
}
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
​
function _isActiveRiskState(IAttackRegistry.ContractState s) internal pure returns (bool) {
return s == IAttackRegistry.ContractState.UNDER_ATTACK || s == IAttackRegistry.ContractState.PROMOTION_REQUESTED;
}
​
function _isTerminalState(IAttackRegistry.ContractState s) internal pure returns (bool) {
return s == IAttackRegistry.ContractState.PRODUCTION || s == IAttackRegistry.ContractState.CORRUPTED;
}

It is checking for _isTerminalState, but it does not check for the outcome

as the outcome can be in a terminal state too

so as a result calling the _markRiskWindowEnd(); is inaccurate

if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
// @ on this code block, it should be checking for the outcome terminal state too

Recommended Mitigation

When checking for the terminal, also check with the outcome too, using PoolStates::isTerminal

Like an example

​
- if (riskWindowEnd == 0 && _isTerminalState(state)) {
+ if ((riskWindowEnd == 0 && PoolStates.isTerminal(outcome)) || (riskWindowEnd == 0 && _isTerminalState(state))) {
_markRiskWindowEnd();
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!