Position Exchange’s Re-Entrancy Loophole Explained
Yet again, a re-entrancy loophole has been identified by the blockchain security team at Amber Group and independent researcher Rivaill , and reported to ImmuneFi. Since no further comments were made by the Position Exchange team after the loophole was fixed, we have decided to publish the details in this blog post.
0x00 Position Exchange
Position Exchange is a decentralized crypto platform which allows users to cast NFTs using POSI tokens and stake NFTs to earn more POSI. As described in this HOWTO document, a user can cast six NFT characters to obtain a randomly generated increase in mining efficiency and mining power.
For example, a user pays 50 POSI to cast this Pilot NFT. The randomly generated mining efficiency is 154.06% and so he/she receives 50 x 154.06% = 76.28 POSI in mining power. If the user chooses to stake this mining power, he/she receives a share of the NFT pool rewards for doing so.
To achieve the NFT staking mechanism mentioned above, the NFTReward contract in the backend implements the stake() and unstake() functions which allow users to transfer in/out ERC721-based NFTs. However, as a result of the native features of the NFTs to the ERC721 token standard, the logic of NFTReward is broken, enabling bad actors to stealthily re-enter the smart contract to make profits.
The aforementioned unstake() function transfers the ERC721 indexed by the given gegoId to the caller, which is the expected behavior. However, we noticed that the safeTransferFrom() function used in line 283, as seen below, has a special callback mechanism embedded, which could be exploited to hijack the control flow. Additionally, some states are updated (line 285–286) after the interaction (line 283), which violates the Checks-Effects-Interactions pattern.
Specifically, in the OpenZeppelin’s ERC721 implementation, while transfering a ERC721, the onERC721Received() external function is invoked when the receiving address is a contract (line 395 below). This is the key to re-enter the vulnerable contract.
Through this, a bad actor can re-enter any external function to NFTReward. How? We noticed that the following two lines are the only two update operations after the safeTransferFrom() call in the unstake() function:
Both of them are set in the stake() function as shown in the code snippet below.
We believe that the original design was to cache the balance and weight of a specific NFT indexed by gegoId. Later on, if the user chooses to unstake() that NFT, the cached values would be used to update the global _weightBalances and _degoBalances associated with that user (line 276 & 280 below).
So, if the cached balance and weight are corrupted right before those values are used to update the global states, _weightBalances and _degoBalances can be tampered with. In addition to this, we noticed that the _weightBalances values are used to calculate the earned rewards. This means that the tampered with states could be used to gain extra rewards and even drain the reward pool.
The figure below shows six steps to tamper with the _weightBalances of a specific user:
1) the attacker starts from a normal stake() with an exploit contract. As shown in the figure, the exploit contract implements an external function, onERC721Received(), for the later hijacking;
2) the attacker triggers unstake() from that exploit contract;
3) via safeTransferFrom(), the Exp contract successfully hijacks the unstake() call;
4) the Exp contract re-enters the stake() function for enabling the second unstake() call;
5) this crucial step reset _stakeWeightes when Exp returns to the hijacked unstake() call;
6) the attacker issues the second unstake() and tamper with the _weightBalances with the zeroed _stakeWeightes.
Using the steps above, an attacker could keep accumulating _weightBalances with only one NFT and claim rewards through the harvest function with zero NFTs staked in the NFTReward contract. The code snippet of the exploit contract below shows how we simulate the attack. In the trigger function, we perform multiple runs of [stake(), unstake(), unstake()] calls. Whenever unstake() transfers an NFT back to the exploit contract, the onERC721Received() function hijacks the unstake() and issues another stake() call when the flag is set. That’s how we embed the stake() call inside the body of the unstake() call and tamper with the states.
The following eth-brownie screenshot proves our theory of corrupting states and making profits with zero NFTs staked. Specifically, we 1) “borrow” the NFT 1183410; 2) create fabricated states on the NFTReward contract; 3) “return” the NFT; 4) harvest rewards. In the end, our hacker walks away with 4.93 POSI without staking anything in the NFTReward contract. This demonstrates how a bad actor could create multiple contracts, fabricate states for each of them, and harvest rewards by transferring one NFT back and forth.
0x03 Timeline & Mitigation
The loophole was submitted on March 9th through ImmuneFi. On March 21st, the Position Exchange team confirmed the issue and rated it as a high severity loophole. On March 22nd, we noticed that a new NFTReward contract was deployed with re-entrancy guards. However, we haven’t received a response from the Position Exchange team since then.
The complete timeline is shown as follows:
Besides Position Exchange, we also contacted Dego Finance and Smarty Pay. Both of them had deployed similar NFTReward contracts that contained similar loopholes. Dego Finance’s team addressed the issue by migrating the DEGO token to a new token. Meanwhile the Smarty Pay team upgraded their NFTReward contract and migrated all states to the new version . As far as we know, no real attack occurred on all three vulnerable projects.
The information contained in this post (the “Information”) has been prepared solely for informational purposes, is in summary form, and does not purport to be complete. The Information is not, and is not intended to be, an offer to sell, or a solicitation of an offer to purchase, any securities. The Information does not provide and should not be treated as giving investment advice. The Information does not take into account specific investment objectives, financial situation or the particular needs of any prospective investor. No representation or warranty is made, expressed or implied, with respect to the fairness, correctness, accuracy, reasonableness or completeness of the Information. We do not undertake to update the Information. It should not be regarded by prospective investors as a substitute for the exercise of their own judgment or research. Prospective investors should consult with their own legal, regulatory, tax, business, investment, financial and accounting advisers to the extent that they deem it necessary, and make any investment decisions based upon their own judgment and advice from such advisers as they deem necessary and not upon any view expressed herein