Sherlock

Staker funds are used as support for all protocols supported by Sherlock. Therefore, adding a new protocol may introduce a risk that one or more *stakers* are not comfortable with. For example, a *staker* might know the team behind a certain protocol that is newly added and believe that the risk of a hack of that protocol is too high to tolerate. Therefore, *stakers* should get a chance to withdraw their funds before a protocol starts being covered by Sherlock.

When calling the `Payout.payout` function the governance address can choose the amount that is to be transferred from the `firstMoneyOut` pool for each token. This goes against the specification, which indicates that the `firstMoneyOut` pool will be emptied first in case of a valid claim.

The documentation states that: > Sherlock implements a buffer amount (20% currently) to account for currency fluctuations. So if Sherlock will cover a protocol for $100M, the staking pools need to be at least $120M in size ($100M we need to cover + 20% buffer). However, given that we have seen price drops of more than 50% in the past couple of months, this 20% buffer seems insufficient.

The Sherlock pool must hold at least the amount necessary to payout the largest protocol, plus a 20% buffer. However, if a large *staker* decides to withdraw their stake at some point and the size of the pool decreases below the amount needed by one or more of the largest platforms, then those platforms will overpay the amount of premium from that point onward until either the premium amount is adjusted by the governance or sufficient funds are staked to restore the pool size to the amount needed to cover the largest platforms.

As it presently is constructed, the contract is vulnerable to the [allowance double-spend exploit](https://github.com/OpenZeppelin/openzeppelin-solidity/blob/b4f87bb8fc25fb07f73099701e39e167a3d36465/contracts/token/ERC20/ERC20.sol#L71-L78), as with other ERC20 tokens.

The governance can set the cooldown period to 25 million blocks, which (assuming a block time of 13 seconds) is over 10 years. This is an excessive amount of time for stakers to wait before they can withdraw their funds.

The contract does not declare any events, which makes it difficult for 3rd party integrators to monitor the actions performed by the smart contracts. The only exception is the `SherX.sol` file that defines and emits the `Harvest` event.

The following functions are missing input parameter validation: 1. The `GovDev.transferGovDev()` function does not check if the `_govDev` parameter is different from `address(0)`. 2. The `Payment.payment()` method does not check if the same token address is provided twice in the `_tokens` input array parameter. 3. The `_amounts` and `_firstMoneyOut` values are not checked to be less than the available amounts in the pools. There is an implicit check by using SafeMath functions which will fail if these conditions are false, but will not provide a proper error message. 4. The `_amount` input parameter of the `PoolBase.withdrawProtocolBalance` function is not explicitly checked to be less than `ps.protocolBalance[_protocol]`. This could lead to an error in the `sub()` function call on L292, without any error message. 5. The `PoolBase.setCooldownFee` function does not impose an upper limit on the `_fee` parameter value and this fee can be changed at any time by the governance account.

1. From the protocol overview: "Our claims committee will be the final arbiters in deciding whether a certain hack falls under coverage and therefore should be paid out." 2. Within `Gov.sol`, several protocol parameters may be changed, e.g., `unstakeWindow`. 3. The owner is responsible for setting token prices and protocol premiums. **No oracles are used for pricing.** 4. The diamond pattern would allow the owner to add arbitrary code to the solution as facets via `GovDev.updateSolution`. Also, other privileged roles should be documented, which can perform a series of actions that could affect end-users: * The main governance role can do the following: 1. Change the address of the main governance role at any time. 2. Change the Watson's address at any time. 3. Set the length of the unstake window to any value under 10 years, at any time. 4. Set the length of the cooldown period to any value under 10 years, at any time. 5. Add new protocols at any time. 6. Update the protocol agent at any time. 7. Add any ERC20 tokens the protocol is allowed to pay in at any time. 8. Remove protocols that do not have debt. 9. Initialize a new token. 10. Disable any of the added ERC20 tokens with 0 active weight, for staking. 11. Disable any covered protocol if the active premium and active underlying are 0. 12. Sell/swap ERC20 tokens from the pool at any time. 13. Remove ERC20 tokens from the pool if they are not used by stakers or protocols. 14. Set the price of any ERC20 token in USD at any time. 15. Set the premium amounts for a protocol using multiple ERC20 tokens. 16. Change the address of the payout governance, at any time. 17. Set initial SHERX distribution to Watsons, once. 18. Set the SHERX distribution (weights) to different ERC20 tokens at any time. * The GovDev role can do the following: 19. Set the initial main governance address, once. 20. Change the GovDev address, at any time. 21. Delete, update or add functions to the Sherlock smart contract. 22. Set the initial governance payout address, once. 23. Initialize the SherXERC20 token, once. 24. Stake in the `PoolDevOnly` contract. No other role can stake in this pool.

The `Manager.setProtocolPremium` and `Manager.setProtocolPremiumAndTokenPrice` functions are used to change the premium amount for one or more protocols. These functions all make a call to `LibPool.payOffDebtAll` which pays off the accrued debt of all whitelisted protocols that use a specific token. However, if any of these protocols do not have sufficient balance for the debt to be paid off, it will cause the whole transaction to revert and the protocol premium price will remain unchanged (for any unrelated protocol).