search

code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

Useless reentrancy guards #167

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pedroais

Vulnerability details

Impact

Gas wasting

Proof of Concept

All external functions have reentrancy guards but the contract doesn't have external calls (excepting those to the XDEFI token which is an immutable address).

This means there is no possibility of calling an arbitrary external contract so reentrancy is impossible.

deluca-mike commented 2 years ago

Unfortunately, ERC721 safeMint calls the receiver, if it is a contract, so it can re-enter. And that is exactly the explicit that many found, which was to call updateDistribution() upon receiving an NFT position.

Ivshti commented 2 years ago

it can be argued that because of strict following of check-effect-interaction, reentrancy guards are useless here

even when looping, as long as local logic follows this pattern, the code should be reentrancy safe

I would imagine the sponsor prefers to keep the guards rather than taking the risk and analyzing all the possible cases where safeMint can reenter, so I am going to agree with the dispute