QA Report - Githubissues

Summary

Low Risk Issues

	Issue	Instances
1	`migrate()` still does transfers when the transfer is to the same pool, and this can be done multiple times	1
2	Non-exploitable reentrancy	1
3	Users can DOS themselves by executing `prepareUnlock(0)` many times	1
4	Unused/empty `receive()`/`fallback()` function	3
5	`safeApprove()` is deprecated	4
6	Missing checks for `address(0x0)` when assigning values to `address` state variables	8
7	`_prepareDeadline()`, `_setConfig()`, and `_executeDeadline()` should be `private`	1

Total: 19 instances over 7 issues

Non-critical Issues

	Issue	Instances
1	Unneded import	1
2	Return values of `approve()` not checked	1
3	Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability	2
4	Missing event for critical parameter change	3
5	Use a more recent version of solidity	1
6	Use a more recent version of solidity	16
7	Constant redefined elsewhere	10
8	Inconsistent spacing in comments	3
9	File is missing NatSpec	5
10	NatSpec is incomplete	17
11	Event is missing `indexed` fields	10
12	Not using the named return variables anywhere in the function is confusing	2
9	Typos	6

Total: 80 instances over 13 issues

Low Risk Issues

1. `migrate()` still does transfers when the transfer is to the same pool, and this can be done multiple times

There's no check that the old address isn't the same as the new address, and there's no check that the migration has already happened

There is 1 instance of this issue:

File: protocol/contracts/zaps/PoolMigrationZap.sol   #1

52       function migrate(address oldPoolAddress_) public override {
53           ILiquidityPool oldPool_ = ILiquidityPool(oldPoolAddress_);
54           IERC20 lpToken_ = IERC20(oldPool_.getLpToken());
55           uint256 lpTokenAmount_ = lpToken_.balanceOf(msg.sender);
56           require(lpTokenAmount_ != 0, "No LP Tokens");
57           require(oldPool_.getWithdrawalFee(msg.sender, lpTokenAmount_) == 0, "withdrawal fee not 0");
58:          lpToken_.safeTransferFrom(msg.sender, address(this), lpTokenAmount_);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L52-L58

2. Non-exploitable reentrancy

There is no reentrancy guard in this function, and if used with a token that has transfer callbacks, such as an ERC777, the caller can reenter before balances is updated. I don't currently see a way to exploit this

There is 1 instance of this issue:

File: protocol/contracts/tokenomics/AmmGauge.sol   #1

130          uint256 oldBal = IERC20(ammToken).balanceOf(address(this));
131          IERC20(ammToken).safeTransfer(dst, amount);
132          uint256 newBal = IERC20(ammToken).balanceOf(address(this));
133          uint256 unstaked = oldBal - newBal;
134          balances[msg.sender] -= unstaked;
135:         totalStaked -= unstaked;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L130-L135

3. Users can DOS themselves by executing `prepareUnlock(0)` many times

There's no check on the amount, and every call add another entry to an array. When the user finally calls executeUnlocks() they'll run out of gas

There is 1 instance of this issue:

File: protocol/contracts/BkdLocker.sol   #1

118      function prepareUnlock(uint256 amount) external override {
119          require(
120              totalStashed[msg.sender] + amount <= balances[msg.sender],
121              "Amount exceeds locked balance"
122:         );

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L118-L122

4. Unused/empty `receive()`/`fallback()` function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))

There are 3 instances of this issue:

File: protocol/contracts/zaps/PoolMigrationZap.sol   #1

31:       receive() external payable {}

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L31

File: protocol/contracts/RewardHandler.sol   #2

30:       receive() external payable {}

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/RewardHandler.sol#L30

File: protocol/contracts/tokenomics/FeeBurner.sol   #3

35:       receive() external payable {} // Recieve function for withdrawing from Backd ETH Pool

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L35

5. `safeApprove()` is deprecated

Deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance(). If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance() can be used instead

There are 4 instances of this issue:

File: protocol/contracts/zaps/PoolMigrationZap.sol   #1

27:               IERC20(underlying_).safeApprove(address(newPool_), type(uint256).max);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L27

File: protocol/contracts/RewardHandler.sol   #2

52:           IERC20(targetLpToken).safeApprove(address(bkdLocker), burnedAmount);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/RewardHandler.sol#L52

File: protocol/contracts/RewardHandler.sol   #3

64:           IERC20(token).safeApprove(spender, type(uint256).max);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/RewardHandler.sol#L64

File: protocol/contracts/tokenomics/FeeBurner.sol   #4

118:          IERC20(token_).safeApprove(spender_, type(uint256).max);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L118

6. Missing checks for `address(0x0)` when assigning values to `address` state variables

There are 8 instances of this issue:

File: protocol/contracts/StakerVault.sol

72:           token = _token;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L72

File: protocol/contracts/BkdLocker.sol

49:           rewardToken = _rewardToken;

74:           rewardToken = newRewardToken;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L49

File: protocol/contracts/tokenomics/VestedEscrowRevocable.sol

43:           treasury = treasury_;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrowRevocable.sol#L43

File: protocol/contracts/tokenomics/AmmGauge.sol

39:           ammToken = _ammToken;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L39

File: protocol/contracts/tokenomics/VestedEscrow.sol

65:           fundAdmin = fundAdmin_;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L65

File: protocol/contracts/tokenomics/KeeperGauge.sol

48:           pool = _pool;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/KeeperGauge.sol#L48

File: protocol/contracts/tokenomics/BkdToken.sol

21:           minter = _minter;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/BkdToken.sol#L21

7. `_prepareDeadline()`, `_setConfig()`, and `_executeDeadline()` should be `private`

I flagged this in the last Backd contest, but it doesn't seem to have been addressed, so bringing it up again: These functions have the ability to bypass the timelocks of every setting. No contract besides the Preparable contract itself should need to call these functions, and having them available will lead to exploits. The contracts that currently call _setConfig() in their constructors should be given a new function _initConfig() for this purpose. The Vault calls some of these functions as well, and should be changed to manually inspect the deadline rather than mucking with the internals, which is error-prone. The mappings should also be made private, and there should be public getters to read their values

There is 1 instance of this issue:

File: protocol/contracts/utils/Preparable.sol   #1

115      /**
116       * @notice Execute uint256 config update (with time delay enforced).
117       * @dev Needs to be called after the update was prepared. Fails if called before time delay is met.
118       * @return New value.
119       */
120      function _executeUInt256(bytes32 key) internal returns (uint256) {
121          _executeDeadline(key);
122          uint256 newValue = pendingUInts256[key];
123          _setConfig(key, newValue);
124          return newValue;
125      }
126  
127      /**
128       * @notice Execute address config update (with time delay enforced).
129       * @dev Needs to be called after the update was prepared. Fails if called before time delay is met.
130       * @return New value.
131       */
132      function _executeAddress(bytes32 key) internal returns (address) {
133          _executeDeadline(key);
134          address newValue = pendingAddresses[key];
135          _setConfig(key, newValue);
136          return newValue;
137:     }

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/Preparable.sol#L115-L137

Non-critical Issues

1. Unneded import

There is 1 instance of this issue:

File: protocol/contracts/tokenomics/BkdToken.sol   #1

8:   import "../../libraries/ScaledMath.sol";

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/BkdToken.sol#L8

2. Return values of `approve()` not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

There is 1 instance of this issue:

File: protocol/contracts/tokenomics/VestedEscrow.sol   #1

25:           IERC20(rewardToken_).approve(msg.sender, type(uint256).max);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L25

3. Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability

There are 2 instances of this issue:

File: protocol/contracts/utils/CvxMintAmount.sol   #1

10:       uint256 private constant _CLIFF_SIZE = 100000 * 1e18; //new cliff every 100,000 tokens

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol#L10

File: protocol/contracts/utils/CvxMintAmount.sol   #2

12:       uint256 private constant _MAX_SUPPLY = 100000000 * 1e18; //100 mil max supply

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol#L12

4. Missing event for critical parameter change

There are 3 instances of this issue:

File: protocol/contracts/tokenomics/InflationManager.sol   #1

58        function setMinter(address _minter) external override onlyGovernance returns (bool) {
59            require(minter == address(0), Error.ADDRESS_ALREADY_SET);
60            require(_minter != address(0), Error.INVALID_MINTER);
61            minter = _minter;
62            return true;
63:       }

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L58-L63

File: protocol/contracts/tokenomics/VestedEscrow.sol   #2

68        function setAdmin(address _admin) external override {
69            require(_admin != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
70            require(msg.sender == admin, Error.UNAUTHORIZED_ACCESS);
71            admin = _admin;
72:       }

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L68-L72

File: protocol/contracts/tokenomics/VestedEscrow.sol   #3

74        function setFundAdmin(address _fundadmin) external override {
75            require(_fundadmin != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);
76            require(msg.sender == admin, Error.UNAUTHORIZED_ACCESS);
77            fundAdmin = _fundadmin;
78:       }

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L74-L78

5. Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

There is 1 instance of this issue:

File: protocol/contracts/tokenomics/InflationManager.sol   #1

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L2

6. Use a more recent version of solidity

Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

There are 16 instances of this issue:

File: protocol/contracts/StakerVault.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L2

File: protocol/contracts/Controller.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/Controller.sol#L2

File: protocol/contracts/utils/CvxMintAmount.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol#L2

File: protocol/contracts/BkdLocker.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L2

File: protocol/contracts/zaps/PoolMigrationZap.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L2

File: protocol/contracts/AddressProvider.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/AddressProvider.sol#L2

File: protocol/contracts/RewardHandler.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/RewardHandler.sol#L2

File: protocol/contracts/tokenomics/Minter.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L2

File: protocol/contracts/tokenomics/VestedEscrowRevocable.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrowRevocable.sol#L2

File: protocol/contracts/tokenomics/AmmGauge.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L2

File: protocol/contracts/tokenomics/VestedEscrow.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L2

File: protocol/contracts/tokenomics/KeeperGauge.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/KeeperGauge.sol#L2

File: protocol/contracts/tokenomics/FeeBurner.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L2

File: protocol/contracts/tokenomics/LpGauge.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/LpGauge.sol#L2

File: protocol/contracts/tokenomics/BkdToken.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/BkdToken.sol#L2

File: protocol/contracts/access/RoleManager.sol

2:    pragma solidity 0.8.10;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/access/RoleManager.sol#L2

7. Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

There are 10 instances of this issue:

File: protocol/contracts/Controller.sol

/// @audit seen in protocol/contracts/StakerVault.sol 
21:       IAddressProvider public immutable override addressProvider;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/Controller.sol#L21

File: protocol/contracts/RewardHandler.sol

/// @audit seen in protocol/contracts/StakerVault.sol 
20:       IController public immutable controller;

/// @audit seen in protocol/contracts/Controller.sol 
21:       IAddressProvider public immutable addressProvider;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/RewardHandler.sol#L20

File: protocol/contracts/tokenomics/Minter.sol

/// @audit seen in protocol/contracts/RewardHandler.sol 
55:       IController public immutable controller;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L55

File: protocol/contracts/tokenomics/InflationManager.sol

/// @audit seen in protocol/contracts/RewardHandler.sol 
24:       IAddressProvider public immutable addressProvider;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L24

File: protocol/contracts/tokenomics/AmmGauge.sol

/// @audit seen in protocol/contracts/tokenomics/Minter.sol 
20:       IController public immutable controller;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L20

File: protocol/contracts/tokenomics/KeeperGauge.sol

/// @audit seen in protocol/contracts/tokenomics/AmmGauge.sol 
30:       IController public immutable controller;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/KeeperGauge.sol#L30

File: protocol/contracts/tokenomics/LpGauge.sol

/// @audit seen in protocol/contracts/tokenomics/KeeperGauge.sol 
19:       IController public immutable controller;

/// @audit seen in protocol/contracts/StakerVault.sol 
21:       IInflationManager public immutable inflationManager;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/LpGauge.sol#L19

File: protocol/contracts/access/RoleManager.sol

/// @audit seen in protocol/contracts/tokenomics/InflationManager.sol 
25:       IAddressProvider public immutable addressProvider;

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/access/RoleManager.sol#L25

8. Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There are 3 instances of this issue:

File: protocol/contracts/utils/CvxMintAmount.sol   #1

11:       uint256 private constant _CLIFF_COUNT = 1000; // 1,000 cliffs

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol#L11

File: protocol/contracts/utils/CvxMintAmount.sol   #2

14:           IERC20(address(0x4e3FBD56CD56c3e72c1403e103b45Db9da5B9D2B)); // CVX Token

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol#L14

File: protocol/contracts/tokenomics/InflationManager.sol   #3

532:      //TOOD: See if this is still needed somewhere

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L532

9. File is missing NatSpec

There are 5 instances of this issue:

File: protocol/contracts/utils/CvxMintAmount.sol

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/CvxMintAmount.sol

File: protocol/contracts/tokenomics/VestedEscrowRevocable.sol

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrowRevocable.sol

File: protocol/contracts/tokenomics/VestedEscrow.sol

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol

File: protocol/contracts/access/Authorization.sol

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/access/Authorization.sol

File: protocol/contracts/access/RoleManager.sol

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/access/RoleManager.sol

10. NatSpec is incomplete

There are 17 instances of this issue:

File: protocol/contracts/StakerVault.sol

/// @audit Missing: '@param strategy'
93        /**
94         * @notice Registers an address as a strategy to be excluded from token accumulation.
95         * @dev This should be used if a strategy deposits into a stakerVault and should not get gov. tokens.
96         * @return `true` if success.
97         */
98:       function addStrategy(address strategy) external override returns (bool) {

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/StakerVault.sol#L93-L98

File: protocol/contracts/Controller.sol

/// @audit Missing: '@param payer'
117       /**
118        * @return the total amount of ETH require by `payer` to cover the fees for
119        * positions registered in all actions
120        */
121:      function getTotalEthRequiredForGas(address payer) external view override returns (uint256) {

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/Controller.sol#L117-L121

File: protocol/contracts/utils/Preparable.sol

/// @audit Missing: '@param key'
33        /**
34         * @notice Prepares an uint256 that should be committed to the contract
35         * after `_MIN_DELAY` elapsed
36         * @param value The value to prepare
37         * @return `true` if success.
38         */
39        function _prepare(
40            bytes32 key,
41            uint256 value,
42            uint256 delay
43:       ) internal returns (bool) {

/// @audit Missing: '@param delay'
33        /**
34         * @notice Prepares an uint256 that should be committed to the contract
35         * after `_MIN_DELAY` elapsed
36         * @param value The value to prepare
37         * @return `true` if success.
38         */
39        function _prepare(
40            bytes32 key,
41            uint256 value,
42            uint256 delay
43:       ) internal returns (bool) {

/// @audit Missing: '@param key'
57        /**
58         * @notice Prepares an address that should be committed to the contract
59         * after `_MIN_DELAY` elapsed
60         * @param value The value to prepare
61         * @return `true` if success.
62         */
63        function _prepare(
64            bytes32 key,
65            address value,
66            uint256 delay
67:       ) internal returns (bool) {

/// @audit Missing: '@param delay'
57        /**
58         * @notice Prepares an address that should be committed to the contract
59         * after `_MIN_DELAY` elapsed
60         * @param value The value to prepare
61         * @return `true` if success.
62         */
63        function _prepare(
64            bytes32 key,
65            address value,
66            uint256 delay
67:       ) internal returns (bool) {

/// @audit Missing: '@param key'
81        /**
82         * @notice Reset a uint256 key
83         * @return `true` if success.
84         */
85:       function _resetUInt256Config(bytes32 key) internal returns (bool) {

/// @audit Missing: '@param key'
93        /**
94         * @notice Reset an address key
95         * @return `true` if success.
96         */
97:       function _resetAddressConfig(bytes32 key) internal returns (bool) {

/// @audit Missing: '@param key'
115       /**
116        * @notice Execute uint256 config update (with time delay enforced).
117        * @dev Needs to be called after the update was prepared. Fails if called before time delay is met.
118        * @return New value.
119        */
120:      function _executeUInt256(bytes32 key) internal returns (uint256) {

/// @audit Missing: '@param key'
127       /**
128        * @notice Execute address config update (with time delay enforced).
129        * @dev Needs to be called after the update was prepared. Fails if called before time delay is met.
130        * @return New value.
131        */
132:      function _executeAddress(bytes32 key) internal returns (address) {

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/utils/Preparable.sol#L33-L43

File: protocol/contracts/AddressProvider.sol

/// @audit Missing: '@return'
79         * @param action Address of action to add.
80         */
81:       function addAction(address action) external override onlyGovernance returns (bool) {

/// @audit Missing: '@param freezable'
207       /**
208        * @notice Initializes an address
209        * @param key Key to initialize
210        * @param initialAddress Address for `key`
211        */
212       function initializeAddress(
213           bytes32 key,
214           address initialAddress,
215           bool freezable
216:      ) public override onlyGovernance {

/// @audit Missing: '@param key'
264       /**
265        * @notice Execute update of `key`
266        * @return New address.
267        */
268:      function executeAddress(bytes32 key) external override returns (address) {

/// @audit Missing: '@param key'
274       /**
275        * @notice Reset `key`
276        * @return true if it was reset
277        */
278:      function resetAddress(bytes32 key) external override onlyGovernance returns (bool) {

/// @audit Missing: '@param token'
396       /**
397        * @notice Tries to get the staker vault for a given token but does not throw if it does not exist
398        * @return A boolean set to true if the vault exists and the vault address.
399        */
400:      function tryGetStakerVault(address token) external view override returns (bool, address) {

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/AddressProvider.sol#L79-L81

File: protocol/contracts/tokenomics/InflationManager.sol

/// @audit Missing: '@param lpToken'
236       /**
237        * @notice Execute update of lp pool weight (with time delay enforced).
238        * @dev Needs to be called after the update was prepared. Fails if called before time delay is met.
239        * @return New lp pool weight.
240        */
241:      function executeLpPoolWeight(address lpToken) external override returns (uint256) {

/// @audit Missing: '@param token'
321       /**
322        * @notice Execute update of lp pool weight (with time delay enforced).
323        * @dev Needs to be called after the update was prepared. Fails if called before time delay is met.
324        * @return New lp pool weight.
325        */
326:      function executeAmmTokenWeight(address token) external override returns (uint256) {

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L236-L241

11. Event is missing `indexed` fields

Each event should use three indexed fields if there are three or more fields

There are 10 instances of this issue:

File: protocol/contracts/zaps/PoolMigrationZap.sol

18:       event Migrated(address user, address oldPool, address newPool, uint256 lpTokenAmount); // Emitted when a migration is completed

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/zaps/PoolMigrationZap.sol#L18

File: protocol/contracts/tokenomics/Minter.sol

58:       event TokensMinted(address beneficiary, uint256 amount);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/Minter.sol#L58

File: protocol/contracts/tokenomics/InflationManager.sol

43:       event NewKeeperWeight(address indexed pool, uint256 newWeight);

44:       event NewLpWeight(address indexed pool, uint256 newWeight);

45:       event NewAmmTokenWeight(address indexed token, uint256 newWeight);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L43

File: protocol/contracts/tokenomics/VestedEscrowRevocable.sol

34:       event Revoked(address indexed user, uint256 revokedAmount);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrowRevocable.sol#L34

File: protocol/contracts/tokenomics/AmmGauge.sol

34:       event RewardClaimed(address indexed account, uint256 amount);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L34

File: protocol/contracts/tokenomics/VestedEscrow.sol

48:       event Fund(address indexed recipient, uint256 reward);

49:       event Claim(address indexed user, uint256 amount);

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/VestedEscrow.sol#L48

File: protocol/contracts/tokenomics/FeeBurner.sol

29:       event Burned(address targetLpToken, uint256 amountBurned); // Emmited after a successfull burn to target lp token

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L29

12. Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 2 instances of this issue:

File: protocol/contracts/tokenomics/FeeBurner.sol   #1

/// @audit received
43        function burnToTarget(address[] memory tokens_, address targetLpToken_)
44            public
45            payable
46            override
47:           returns (uint256 received)

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L43-L47

File: protocol/contracts/tokenomics/FeeBurner.sol   #2

/// @audit received
96        function _depositInPool(address underlying_, ILiquidityPool pool_)
97            internal
98:           returns (uint256 received)

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L96-L98

13. Typos

There are 6 instances of this issue:

File: protocol/contracts/BkdLocker.sol

/// @audit invlude
173:       * @dev This does not invlude the gov. tokens queued for withdrawal.

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L173

File: protocol/contracts/tokenomics/InflationManager.sol

/// @audit TOOD
532:      //TOOD: See if this is still needed somewhere

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/InflationManager.sol#L532

File: protocol/contracts/tokenomics/FeeBurner.sol

/// @audit Emmited
29:       event Burned(address targetLpToken, uint256 amountBurned); // Emmited after a successfull burn to target lp token

/// @audit successfull
29:       event Burned(address targetLpToken, uint256 amountBurned); // Emmited after a successfull burn to target lp token

/// @audit Recieve
35:       receive() external payable {} // Recieve function for withdrawing from Backd ETH Pool

/// @audit Transfering
84:           // Transfering LP tokens back to sender

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/FeeBurner.sol#L29

code-423n4 / 2022-05-backd-findings

QA Report #109

Summary

Low Risk Issues

Non-critical Issues

Low Risk Issues

1. `migrate()` still does transfers when the transfer is to the same pool, and this can be done multiple times

2. Non-exploitable reentrancy

3. Users can DOS themselves by executing `prepareUnlock(0)` many times

4. Unused/empty `receive()`/`fallback()` function

5. `safeApprove()` is deprecated

6. Missing checks for `address(0x0)` when assigning values to `address` state variables

7. `_prepareDeadline()`, `_setConfig()`, and `_executeDeadline()` should be `private`

Non-critical Issues

1. Unneded import

2. Return values of `approve()` not checked

3. Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability

4. Missing event for critical parameter change

5. Use a more recent version of solidity

6. Use a more recent version of solidity

7. Constant redefined elsewhere

8. Inconsistent spacing in comments

9. File is missing NatSpec

10. NatSpec is incomplete

11. Event is missing `indexed` fields

12. Not using the named return variables anywhere in the function is confusing

13. Typos

1. migrate() still does transfers when the transfer is to the same pool, and this can be done multiple times

2. Non-exploitable reentrancy

3. Users can DOS themselves by executing prepareUnlock(0) many times

4. Unused/empty receive()/fallback() function

5. safeApprove() is deprecated

6. Missing checks for address(0x0) when assigning values to address state variables

7. _prepareDeadline(), _setConfig(), and _executeDeadline() should be private

Non-critical Issues

code-423n4 / 2022-05-backd-findings

QA Report #109

Summary

Low Risk Issues

Non-critical Issues

Low Risk Issues

1. migrate() still does transfers when the transfer is to the same pool, and this can be done multiple times

2. Non-exploitable reentrancy

3. Users can DOS themselves by executing prepareUnlock(0) many times

4. Unused/empty receive()/fallback() function

5. safeApprove() is deprecated

6. Missing checks for address(0x0) when assigning values to address state variables

7. _prepareDeadline(), _setConfig(), and _executeDeadline() should be private

Non-critical Issues

1. Unneded import

2. Return values of approve() not checked

3. Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability

4. Missing event for critical parameter change

5. Use a more recent version of solidity

6. Use a more recent version of solidity

7. Constant redefined elsewhere

8. Inconsistent spacing in comments

9. File is missing NatSpec

10. NatSpec is incomplete

11. Event is missing indexed fields

12. Not using the named return variables anywhere in the function is confusing

13. Typos

1. migrate() still does transfers when the transfer is to the same pool, and this can be done multiple times

2. Non-exploitable reentrancy

3. Users can DOS themselves by executing prepareUnlock(0) many times

4. Unused/empty receive()/fallback() function

5. safeApprove() is deprecated

6. Missing checks for address(0x0) when assigning values to address state variables

7. _prepareDeadline(), _setConfig(), and _executeDeadline() should be private

Non-critical Issues

1. `migrate()` still does transfers when the transfer is to the same pool, and this can be done multiple times

3. Users can DOS themselves by executing `prepareUnlock(0)` many times

4. Unused/empty `receive()`/`fallback()` function

5. `safeApprove()` is deprecated

6. Missing checks for `address(0x0)` when assigning values to `address` state variables

7. `_prepareDeadline()`, `_setConfig()`, and `_executeDeadline()` should be `private`

2. Return values of `approve()` not checked

3. Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability

11. Event is missing `indexed` fields