Platform: Code4rena
Start Date: 06/05/2021
Pot Size: $66,000 USDC
Total HM: 16
Participants: 11
Period: 6 days
Judge: cemozer
Total Solo HM: 9
Id: 8
League: ETH
Rank: 8/11
Findings: 2
Award: $3,286.21
🌟 Selected for report: 2
🚀 Solo Findings: 1
0xsomeone
The impact of this finding is difficult to estimate as the contract system within scope is limited in how the various components are meant to be utilized.
A definitive side-effect of this re-entrancy is the delayed application of the afterRedeemHook which, in some implementations, renders NFTs illegible which would not be the case during the re-entrancy's execution. Another side-effect is that the quantity1155 or holdings would be out-of-sync and would indicate the NFT / EIP-1155 token amount to still be "in the system" when it is not.
The safeTransferFrom implementations of both ERC1155 and ERC721 in withdrawNFTsTo contain a callback hook on the recipient of the transfer in case they are a contract as the standard dictates that smart contract recipients should be aware of the transfer.
While re-entrancies are prevented via the nonReentrant modifier for most system functions, they are not done so for swapTo (and consequently swap) invocations meaning that it is still possible to re-enter the system at this stage. Additionally, re-entrancy is still possible in other segments of the codebase i.e. ones that rely on the eligibility contracts.
Manual Review.
The afterRedeemHook paradigm should be changed to a beforeRedeemHook paradigm to ensure that all state changes are applied prior to external calls according to the Checks-Effects-Interactions pattern. Additionally, the state changes within withdrawNFTsTo should occur prior to the safeTransferFrom invocations.
#0 - 0xKiwi
2021-05-20T21:53:17Z
We have made swap and swapTo reentrant safe.
🌟 Selected for report: 0xsomeone
0xsomeone
The distribute function of NFTXFeeDistributor has no access control and will invoke a fallback on the fee receivers, meaning that a fee receiver can re-enter via this function to acquire their allocation repeatedly potentially draining the full balance and sending zero amounts to the rest of the recipients.
A smart contract with a malicious receiveRewards function can re-enter the distribute function with the same vault ID thereby causing the exploit.
Manual review.
Re-entrancy protection should be incorporated into the distribute function. I should note that a seemingly innocuous contract can cause this re-entrancy by simply asking the owners of the project to include an upgrade-able contract that is then replaced for a malicious implementation.