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

Low Findings

Author Revealed upon completion

Root + Impact

Description

  • The protocol correctly implements access control, tracks user staking times for bonus calculations, and prevents invalid pools from being created via reverting transactions.

  • This QA report consolidates three minor findings:

    1. The ConfidencePool inherits Ownable but fails to override renounceOwnership(), allowing the sponsor to accidentally brick all administrative functions.

    2. The withdraw() function calls _clampUserSums(), but this is dead code because withdraw() requires riskWindowStart == 0, and _clampUserSums immediately exits if riskWindowStart == 0.

    3. ConfidencePoolFactory.createPool() omits minStake != 0 and expiry <= type(uint32).max validations, relying entirely on the child clone to revert, which provides poor UX for integrators.

// L-01 Root Cause: Missing override in ConfidencePool.sol
// The contract inherits Ownable but fails to disable renounceOwnership()
​
// L-02 Root Cause: ConfidencePool.sol withdraw()
_clampUserSums(msg.sender); // @> Unreachable logic: riskWindowStart is always 0 here
​
// L-03 Root Cause: ConfidencePoolFactory.sol createPool()
// @> Missing minStake != 0 and expiry <= type(uint32).max checks before cloning
​

Risk

Likelihood:

  • These issues require an explicit owner mistake (L-01), or they are informational / gas-related .



Impact:

  • bug-01 could lead to a permanent loss of administrative control, trapping swept funds if the recovery address is compromised.

  • bug-02 causes a minor gas inefficiency on every withdrawal.

  • bug-03 causes minor UX friction for integrators by returning a child contract error instead of a factory error.

Proof of Concept

The issues are self-evident in the contract logic and architecture.

// L-01: Owner accidentally calls renounceOwnership()
pool.renounceOwnership();
// Admin functions like pause(), setRecoveryAddress(), and setPoolScope() are permanently bricked.
​
// L-02: withdraw() always calls _clampUserSums() with riskWindowStart == 0
function withdraw() {
if (riskWindowStart != 0) revert(); // Enforces riskWindowStart == 0
// ...
_clampUserSums(msg.sender); // Costs ~200 gas but immediately returns inside the function
}
​

Recommended Mitigation

- remove this code
+ add this code
​
// L-01 Mitigation: Override renounceOwnership in ConfidencePool.sol
+ function renounceOwnership() public view override {
+ revert("Ownership renunciation is not supported");
+ }
​
// L-02 Mitigation: Remove dead code in ConfidencePool.sol withdraw()
- _clampUserSums(msg.sender);
​
// L-03 Mitigation: Add checks to ConfidencePoolFactory.sol createPool()
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
+ if (expiry > type(uint32).max) revert ExpiryTooFar();
+ if (minStake == 0) revert InvalidAmount();
​

Support

FAQs

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

Give us feedback!