# 29213 - \[SC - High] The function always revert if \_stakeNFT True d... Submitted on Mar 10th 2024 at 20:34:07 UTC by @stiglitz for [Boost | ZeroLend](https://immunefi.com/bounty/zerolend-boost/) Report ID: #29213 Report type: Smart Contract Report severity: High Target: Impacts: * Permanent freezing of funds * Temporary freezing of funds for at least 1 hour ## Description ## Brief/Intro See the function `safeTransferFrom::_createLock`: ```solidity if (_stakeNFT) { _mint(address(this), _tokenId); bytes memory data = abi.encode(_to); safeTransferFrom(address(this), address(staking), _tokenId, data); } else _mint(_to, _tokenId); ``` if `_stakeNFT == True`, the tx always reverts because `msg.sender` is the user who called `createLock`, and the user does not have approval for moving NFT owned by someone else. As we can see in the code, the NFT is minted to `address(this)`, so the user is not the owner, and he needs approval from the contact. However, the problem imho is the fact that the NFT is not even owned by a user but by the contract itself. Because if approval is implemented to make the transfer succeed, the user has no ability to get what is his. ## Vulnerability Details Let's also look at the `OmnichainStaking::onERC721Received`: ```solidity if (msg.sender == address(lpLocker)) { lpPower[tokenId] = lpLocker.balanceOfNFT(tokenId); _mint(from, lpPower[tokenId] * 4); } ``` And also the function `OmnichainStaking::unstakeLP`: ```solidity function unstakeLP(uint256 tokenId) external { / _burn(msg.sender, lpPower[tokenId] * 4); lpLocker.safeTransferFrom(address(this), msg.sender, tokenId); } ``` Lets say we have the approval and we create a lock with `_stakeNFT == True`. The function `safeTransferFrom` is executed where `from == address(this)` which is `BaseLocker` (`LockerLP`). NOT A USER. Who is gonna call `unstakeLP` ? The tokens are owner by the `LockerLP` contract, and only the contract can unstake them. ## Impact Details The protocol does not work as it is supposed to, and if the function with `_stakeNFT == True` is called and it works (it does not right now) - The user will basically lose his invested money into veNFT and the voting power will be useless owned by the protocol contract. I think the impact is very high and I would say critical. But because of the missing approval, the function does not work at all, so the impact is not there :D ## References Add any relevant links to documentation or code ## Proof of Concept ### Ve mock token ```solidity pragma solidity ^0.8.6; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; // Example class - a mock class derived from ERC20 contract VeToken is ERC20 { constructor(uint256 initialBalance) ERC20("Ve Token", "VT") public { _mint(msg.sender, initialBalance); } } ``` ### Re mock token ```solidity pragma solidity ^0.8.6; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; // Example class - a mock class derived from ERC20 contract ReToken is ERC20 { constructor(uint256 initialBalance) ERC20("Re Token", "RT") public { _mint(msg.sender, initialBalance); } } ``` ### X contract (ERC721 compatible) ```solidity import {ILocker} from "../contracts/interfaces/ILocker.sol"; import {OmnichainStaking} from "../contracts/locker/OmnichainStaking.sol"; import {PoolVoter} from "../contracts/voter/PoolVoter.sol"; contract X { ILocker public lpLocker; OmnichainStaking public staking; PoolVoter public poolVoter; constructor(address _lpLocker, address _staking, address _poolVoter){ lpLocker = ILocker(_lpLocker); staking = OmnichainStaking(_staking); poolVoter = PoolVoter(_poolVoter); } function onERC721Received( address, address from, uint256 tokenId, bytes calldata data ) external returns (bytes4) { //lpLocker.safeTransferFrom(address(this), lpLocker, tokenId); return this.onERC721Received.selector; } // Unstake from omnichain staking function unstakeLP(uint256 tokenId) external { staking.unstakeLP(tokenId); } // This allows me to send anywhere I want function send(address to, uint256 tokenId) external { lpLocker.safeTransferFrom(address(this), to, tokenId); } function vote(address[] calldata _poolVote,uint256[] calldata _weights) external { poolVoter.vote(_poolVote, _weights); } } ``` ### Y contract (ERC721 compatible) ```solidity import {ILocker} from "../contracts/interfaces/ILocker.sol"; import {OmnichainStaking} from "../contracts/locker/OmnichainStaking.sol"; import {PoolVoter} from "../contracts/voter/PoolVoter.sol"; contract Y { ILocker public lpLocker; OmnichainStaking public staking; PoolVoter public poolVoter; constructor(address _lpLocker, address _staking, address _poolVoter){ lpLocker = ILocker(_lpLocker); staking = OmnichainStaking(_staking); poolVoter = PoolVoter(_poolVoter); } function onERC721Received( address, address from, uint256 tokenId, bytes calldata data ) external returns (bytes4) { //lpLocker.safeTransferFrom(address(this), lpLocker, tokenId); return this.onERC721Received.selector; } // Unstake from omnichain staking function unstakeLP(uint256 tokenId) external { staking.unstakeLP(tokenId); } // This allows me to send anywhere I want function send(address to, uint256 tokenId) external { lpLocker.safeTransferFrom(address(this), to, tokenId); } function vote(address[] calldata _poolVote,uint256[] calldata _weights) external { poolVoter.vote(_poolVote, _weights); } } ``` ### Failing test ```python from wake.testing import * from pytypes.openzeppelin.contracts.proxy.ERC1967.ERC1967Proxy import ERC1967Proxy from pytypes.contracts.locker.OmnichainStaking import OmnichainStaking from pytypes.contracts.locker.LockerToken import LockerToken from pytypes.contracts.voter.PoolVoter import PoolVoter from pytypes.contracts.locker.LockerLP import LockerLP from pytypes.tests.ReToken import ReToken from pytypes.tests.VeToken import VeToken from pytypes.tests.X import X from pytypes.tests.Y import Y ''' Test written in Wake testing framework (https://getwake.io/) aka boosted brownie Docs: https://ackeeblockchain.com/wake/docs/latest/ Repo: https://github.com/Ackee-Blockchain/wake How to run this test: Install wake $ pip install eth-wake To have actual anvil version $ foundryup After installing project dependencies initialize wake It will create `tests` folder and process foundry remappings if any $ wake up Generate python representation of contracts $ wake init pytypes Go to wake `tests` folder and paste this code in tests/test_approval.py and run $ wake test tests/test_approval.py If you are interest I would be happy to provide more examples + complete protocol deployment and fuzz testing (yout tests are not good tbh) ''' def deploy_with_proxy(contract): impl = contract.deploy() proxy = ERC1967Proxy.deploy(impl, b"") return contract(proxy) # Print failing tx call trace def revert_handler(e: TransactionRevertedError): if e.tx is not None: print(e.tx.call_trace) @default_chain.connect() @on_revert(revert_handler) def test_approval(): # ======================DEPLOY========================= # random = default_chain.accounts[9] owner = default_chain.accounts[0] bob = default_chain.accounts[1] omnichain = deploy_with_proxy(OmnichainStaking) locker = deploy_with_proxy(LockerLP) pool_voter = deploy_with_proxy(PoolVoter) # Two mock tokens - underlying for Locker and reward for PoolVoter ve_token = VeToken.deploy(100*10**18, from_=bob) re_token = ReToken.deploy(100*10**18, from_=bob) omnichain.init(random, random, locker, from_=owner) locker.init(ve_token, omnichain, random, from_=owner) pool_voter.init(omnichain, re_token, from_=owner) # Deploy two contracts with the ability to receive and send ERC721 # Both controlled by Bob x = X.deploy(locker, omnichain, pool_voter, from_ = bob) y = Y.deploy(locker, omnichain, pool_voter, from_ = bob) # Random addresse for gauge and asset are OK now gauge = default_chain.accounts[2] asset = default_chain.accounts[3] pool_voter.registerGauge(asset, gauge, from_=owner) # ===================================================== # # Lock time two_weeks = 60*60*24*14 # Amount amount = 10*10**18 # Bob approve locker contract ve_token.approve(locker, amount, from_=bob) # Bob creates lock for X locker.createLockFor(amount, two_weeks,x ,True, from_=bob) # This tx with _stakeNFT == True ALWAYS FAILS # ===================================================== # ``` --- # Agent Instructions: Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter: ``` GET https://reports.immunefi.com/zerolend/29213-sc-high-the-function-always-revert-if-_stakenft-true-d....md?ask= ``` The question should be specific, self-contained, and written in natural language. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.