Dangerous external calls in the middle of various state changes could cause reentrancy issues since there is no reentrancy guard in any functions.
Proof of Concept
When users call the deposit or sponsor functions a deposit NFT is minted. The _safeMint() function that's used makes an unsafe external call to the receiver contract that could reenter any function of the protocol.
naps62
commented
2 years ago
Handle
pedroais
Vulnerability details
Impact
Dangerous external calls in the middle of various state changes could cause reentrancy issues since there is no reentrancy guard in any functions.
Proof of Concept
When users call the deposit or sponsor functions a deposit NFT is minted. The _safeMint() function that's used makes an unsafe external call to the receiver contract that could reenter any function of the protocol.
https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/vault/Depositors.sol#L53
The safe mint function calls _checkOnERC721Received which will make an external call to the contract receiving the NFT.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/3458c1e8541ce0a0cd935828c9db8f9cbca988a0/contracts/token/ERC721/ERC721.sol#L263
An example of safeMint being used for reentrancy can be found in this blog post by samczsun : https://www.paradigm.xyz/2021/08/the-dangers-of-surprising-code/
Recommended Mitigation Steps
Add reentrancy guard to all public statechanching functions.