Omniscia Colony Lab Audit
AuthorizedStaking Manual Review Findings
AuthorizedStaking Manual Review Findings
ASG-01M: Improper Increase of Registered Tokens
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | AuthorizedStaking.sol:L286 |
Description:
The _restake function improperly increases the registered tokens in case both the from and to address are registered, such as during a self-transfer.
Example:
256/**257 * @notice Restakes tokens with the authorization limitations258 * @dev Covers stake and both restake partial and restake all259 */260function _restake(address from, address to, uint256 amount) internal virtual override {261 // stake conditions for accounts could be different262 require(263 stakeBalanceOf(to) + amount >= accountAuthorizedAmount(to),264 "not enough for authorization"265 );266
267 _decreaseTotalRegisteredTokens(from, amount);268
269 // restake all270 if (amount == stakeBalanceOf(from)) {271 _unregisterAccount(from);272 } else {273 // restake partial274 require(275 stakeBalanceOf(from) >= amount + accountAuthorizedAmount(from),276 "amount breaks authorization"277 );278 }279
280 // save conditions before actual stake,281 // expect: deposits[account] == 0282 _saveInitialConditions(to);283
284 super._restake(from, to, amount);285
286 _increaseTotalRegisteredTokens(to, amount);287}Recommendation:
We advise the accounting system to be corrected as it is currently broken.
Alleviation:
The contract's logic has been relocated to the StakingV2.sol file and has been properly alleviated by introducing an if clause that stops execution if the from and to addresses are equivalent.
ASG-02M: Inexistent Subtraction of Registered Tokens
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | AuthorizedStaking.sol:L222-L235 |
Description:
Whenever an account is unregistered, their tokens remain accounted for in the totalRegisteredStake incorrectly.
Example:
213/**214 * @dev Decrease registered tokens only for registered accounts215 */216function _decreaseTotalRegisteredTokens(address account, uint256 amount) internal {217 if (isAccountRegistered(account)) {218 totalRegisteredStake -= amount;219 }220}221
222/**223 * @notice Makes account unregistered224 * @dev Also deletes account initialStakeConditions225 *226 * Emits a {Unregistered} event for authorized accounts227 */228function _unregisterAccount(address account) internal {229 delete initialStakeConditions[account];230
231 if (isAccountRegistered(account)) {232 registered[account] = false;233 emit Unregistered(account);234 }235}Recommendation:
We advise them to be properly decremented by a corresponding call during the _unregisterAccount function's execution.
Alleviation:
The contract's logic was relocated to the StakingV2.sol file and the logic has been updated to properly decrement the totalRegisteredStake variable thereby alleviating this exhibit.
ASG-03M: Inexplicable Capability of Re-Invocation
| Type | Severity | Location |
|---|---|---|
| Input Sanitization |  | AuthorizedStaking.sol:L335-L344, L369-L374 |
Description:
The migrator and stakingV1Contract variables of the contract are powerful and should be assigned only once.
Example:
335/**336 * @notice Allows to set new migrator address337 * @param newMigrator Address of the new migrator338 *339 * Emits a {MigratorChanged} event340 */341function setMigrator(address newMigrator) external onlyOwner {342 migrator = newMigrator;343 emit MigratorChanged(newMigrator);344}Recommendation:
We advise the functions to prevent re-invocation by imposing a corresponding require check.
Alleviation:
The contract's logic has been relocated to the StakingV2.sol contract and the migrator as well as the stakingV1Contract can both be set once thereby alleviating this exhibit in full.