Backd contest - StyxRave's results

Originally submitted by warden StyxRave in https://github.com/code-423n4/2022-04-backd-findings/issues/153, duplicate of https://github.com/code-423n4/2022-04-backd-findings/issues/49.

• BkdTriHopCvx.sol decimalMultiplier assumes tokens have <= 18 decimals. Will always be 0 for tokens with more than 18 (which would be still ERC20 compliant).

Hey, just some comments on things I understand as best practices / potential future proofing when extending this codebase. Didn't really have the time to go deep this round.

• Controller.sol setInflationManager, addStakerVault and removePool should probably emit events indicating the changes made, as they are sensitive actions governance can make.

• CvxCrvRewardsLocker.sol setTreasury does not check for zero address - uninitialized parameters by human error would end up with burned tokens if followed by any withdraw to treasury.

• GasBank.sol withdrawUnused and withdrawFrom allow any action to withdraw from any account. Currently not a problem because only the TopUpAction exists, but there should probably be some kind of mapping between actions and the accounts they can affect / manage.

• RoleManager.sol External functions are protected by the onlyGovernance modifier and call internal _grantRole / _revokeRole. removeGaugeZap should probably follow the same convention (currently not modifier protected, but instead calls public revokeRole, which is)

• TopUpAction.sol

  TopUpActionLibrary's lockFunds doesn't use the addressProvider pattern existing in the codebase. Nothing ensures that stakerVaultAddress corresponds to token. Not currently dangerous because only TopUpAction::_lockFunds calls this function, appropiately getting the vault address from the addressProvider, but should probably future proof by doing it inside as the other functions in the library.

• BkdTriHopCvx.sol decimalMultiplier assumes tokens have <= 18 decimals. Will always be 0 for tokens with more than 18 (which would be still ERC20 compliant).

• StrategySwapper.sol _decimalMultiplier assumes tokens have <= 18 decimals. Will always be 0 for tokens with more than 18 (which would be still ERC20 compliant).

• InflationManager.sol setMinter does not emit an event whilst changing the allowed minter address (a sensitive, governance protected action).

Cheers!