# Peapods Dex Adapter Update Security Review // 19-04-2024 ### Scope https://github.com/peapodsfinance/contracts/pull/35/ ### Commit hash and branch https://github.com/peapodsfinance/contracts/pull/35/files# ## [M-1] **POTENTIAL**: `_rewardsPerShare` can be inflated ### Summary The very first depositor of `TokenRewards` can manipulate the `_rewardsPerShare` such that the interaction with the `TokenRewards` contract would not be possible for the other users ### Vulnerability Details When the first depositor comes in to TokenRewards contract the shares are set for the very first time: ```solidity= function _addShares(address _wallet, uint256 _amount) internal { . . -> totalShares += _amount; . . } ``` Then, the first depositor can deposit any amount of rewards permissionlessy and since there is only 1 staker all the rewards will be belong to the only staker: ```solidity= function _depositRewards(address _token, uint256 _amountTotal) internal { . . . . -> _rewardsPerShare[_token] += (PRECISION * _depositAmount) / totalShares; . } ``` Now, assume the first depositor only deposits "1" shares and immediately deposits 100 PEAS as reward (100e18). `_rewardsPerShare[PEAS] = 1e18 * 100e18 / 1 = 100e36` as we can observe, the reward per share value is extremely high due to shares being "1". Afterwards, the depositor can immediately withdraw its stake and receive the 100 PEAS back since he was the only depositor. Though, the `rewardPerShare` is still 100e36. **User can replay this to make it bigger such that it overflows? maybe there can be other things go wrong? not sure, will think of it and delete/complete this issue...** ### Links to Code https://github.com/peapodsfinance/contracts/blob/9b5ce1a6848668f18f00460b10bc0b64c6ca81e9/contracts/TokenRewards.sol#L208-L240 ### Impact ?? ### Recommendation ?? - [ ] Fixed - [ ] Will not fix ## Low Severity Issues and Best Practices * Gas save, unnecessary code, can be deleted https://github.com/peapodsfinance/contracts/blob/9b5ce1a6848668f18f00460b10bc0b64c6ca81e9/contracts/DecentralizedIndex.sol#L227-L229 - [ ] Fixed - [ ] Will not fix * Gas save, instead of checking the allowance just set the allowance to "0" directly **Recommendation:** `IERC20(PAIRED_LP_TOKEN).safeApprove(V2_ROUTER, 0);` https://github.com/peapodsfinance/contracts/blob/9b5ce1a6848668f18f00460b10bc0b64c6ca81e9/contracts/DecentralizedIndex.sol#L360-L369 - [ ] Fixed - [ ] Will not fix * Gas save, instead of making an external call here to `PROTOCOL_FEE_ROUTER` use a hardcoded value like "10_000". The denominator most likely never changes and the `PROTOCOL_FEE_ROUTER` can't be changed, so it does not make sense to make an external call to just get the DEN and spend unnecessary gas https://github.com/peapodsfinance/contracts/blob/9b5ce1a6848668f18f00460b10bc0b64c6ca81e9/contracts/TokenRewards.sol#L227 https://github.com/peapodsfinance/contracts/blob/9b5ce1a6848668f18f00460b10bc0b64c6ca81e9/contracts/TokenRewards.sol#L290 - [ ] Fixed - [ ] Will not fix