Omniscia Stakewise Audit
Pool Manual Review Findings
Pool Manual Review Findings
POO-01M: Inexistent Validation of Amount
| Type | Severity | Location |
|---|---|---|
| Logical Fault | Minor | Pool.sol:L209-L219 |
Description:
The activateMultiple function does not validate the amount of a particular validatorIndex, allowing fake Activated events to be emitted along real ones which can cause off-chain processes to misbehave.
Example:
contracts/pool/Pool.sol
185/**186 * @dev See {IPool-activate}.187 */188function activate(address account, uint256 validatorIndex) external override whenNotPaused {189 require(190 validatorIndex.mul(1e4) <= activatedValidators.mul(pendingValidatorsLimit.add(1e4)),191 "Pool: validator is not active yet"192 );193
194 uint256 amount = activations[account][validatorIndex];195 require(amount > 0, "Pool: invalid validator index");196
197 delete activations[account][validatorIndex];198 stakedEthToken.mint(account, amount);199 emit Activated(account, validatorIndex, amount, msg.sender);200}201
202/**203 * @dev See {IPool-activateMultiple}.204 */205function activateMultiple(address account, uint256[] calldata validatorIndexes) external override whenNotPaused {206 uint256 toMint;207 uint256 _activatedValidators = activatedValidators;208 for (uint256 i = 0; i < validatorIndexes.length; i++) {209 uint256 validatorIndex = validatorIndexes[i];210 require(211 validatorIndex.mul(1e4) <= _activatedValidators.mul(pendingValidatorsLimit.add(1e4)),212 "Pool: validator is not active yet"213 );214
215 uint256 amount = activations[account][validatorIndex];216 toMint = toMint.add(amount);217 delete activations[account][validatorIndex];218
219 emit Activated(account, validatorIndex, amount, msg.sender);220 }221 require(toMint > 0, "Pool: invalid validator index");222 stakedEthToken.mint(account, toMint);223}Recommendation:
We strongly recommend the code of activate to be refactored to invoke an internal _activate function that yields the amount of tokens that should be minted and the function to be utilized by both activate and activateMultiple as the activate implementation does impose the proper check on the amount being activated.
Alleviation:
The code was refactored according to our recommendation, utilizing an internal _activateAmount function that performs the equivalent statements of both implementations in a gas-efficient manner.