User can steal rewards from other users by withdrawing their Velo Deposit NFTs from other users' depositors
Rewards from staking AMM tokens accumulate to the depositor used to deposit them. The rewards accumulated by a depositor are passed to the owner when they claim. A malicious user to steal the rewards from other users by manipulating other users depositors. Since any NFT of a DepositReceipt can be withdrawn from any depositor with the same DepositReceipt, a malicious user could mint an NFT on their depositor then withdraw in from another user's depositor. The net effect is that that the victims deposits will effectively be in the attackers depositor and they will collect all the rewards.\\n```\\nfunction withdrawFromGauge(uint256 _NFTId, address[] memory _tokens)  public  {\\n    uint256 amount = depositReceipt.pooledTokens(_NFTId);\\n    depositReceipt.burn(_NFTId);\\n    gauge.getReward(address(this), _tokens);\\n    gauge.withdraw(amount);\\n    //AMMToken adheres to ERC20 spec meaning it reverts on failure, no need to check return\\n    //slither-disable-next-line unchecked-transfer\\n    AMMToken.transfer(msg.sender, amount);\\n}\\n```\\n\\nEvery user must create a `Depositor` using `Templater` to interact with vaults and take loans. `Depositor#withdrawFromGauge` allows any user to withdraw any NFT that was minted by the same `DepositReciept`. This is where the issues arises. Since rewards are accumulated to the `Depositor` in which the underlying is staked a user can deposit to their `Depositor` then withdraw their NFT through the `Depositor` of another user's `Depositor` that uses the same `DepositReciept`. The effect is that the tokens will remained staked to the attackers `Depositor` allowing them to steal all the other user's rewards.\\nExample: `User A` and `User B` both create a `Depositor` for the same `DepositReciept`. Both users deposit 100 tokens into their respective `Depositors`. `User B` now calls `withdrawFromGauge` on `Depositor` A. `User B` gets their 100 tokens back and `Depositor B` still has 100 tokens deposited in it. `User B` cannot steal these tokens but they are now collecting the yield on all 100 tokens via `Depositor B` and `User A` isn't getting any rewards at all because `Depositor` A no longer has any tokens deposited into Velodrome gauge.
Depositors should only be able to burn NFTs that they minted. Change DepositReciept_Base#burn to enforce this:\\n```\\n    function burn(uint256 _NFTId) external onlyMinter{\\n+       //tokens must be burned by the depositor that minted them\\n+       address depositor = relatedDepositor[_NFTId];\\n+       require(depositor == msg.sender, "Wrong depositor");\\n        require(_isApprovedOrOwner(msg.sender, _NFTId), "ERC721: caller is not token owner or approved");\\n        delete pooledTokens[_NFTId];\\n        delete relatedDepositor[_NFTId];\\n        _burn(_NFTId);\\n    }\\n```\\n
Malicious user can steal other user's rewards
```\\nfunction withdrawFromGauge(uint256 _NFTId, address[] memory _tokens)  public  {\\n    uint256 amount = depositReceipt.pooledTokens(_NFTId);\\n    depositReceipt.burn(_NFTId);\\n    gauge.getReward(address(this), _tokens);\\n    gauge.withdraw(amount);\\n    //AMMToken adheres to ERC20 spec meaning it reverts on failure, no need to check return\\n    //slither-disable-next-line unchecked-transfer\\n    AMMToken.transfer(msg.sender, amount);\\n}\\n```\\n