ERC721 tokens callback to the receiver, which makes contracts handling these tokens vulnerable to reentrancy. This is mitigated by putting in the nonReetrant modifier, which is missing in certain functions in this contract. No exploit to steal tokens was found, but this is still classified as a medium since it is a vulnerable coding pattern that can lead to damage by a more creative attacker.
Vulnerability Detail
The attacker contract receives the ERC721 token when the settleContract function is called. This hands over control to the attacker contract before making the following state changes:
Transferring ERC20 tokens to the bear
Marking the contract as settled
The attacker can enter the following functions when it receives the ERC721:
withdrawToken
transferPosition
This is also true when the attacker receives an ERC721 token from the withdrawToken which still has a poending state change:
Mark withdrawableCollectionTokenId to address(0) making it not withdrawable anymore
carrot
medium
Re-entrancy in certain functions
Summary
ERC721 tokens callback to the receiver, which makes contracts handling these tokens vulnerable to reentrancy. This is mitigated by putting in the nonReetrant modifier, which is missing in certain functions in this contract. No exploit to steal tokens was found, but this is still classified as a medium since it is a vulnerable coding pattern that can lead to damage by a more creative attacker.
Vulnerability Detail
The attacker contract receives the ERC721 token when the
settleContractfunction is called. This hands over control to the attacker contract before making the following state changes:The attacker can enter the following functions when it receives the ERC721:
withdrawTokentransferPositionThis is also true when the attacker receives an ERC721 token from the
withdrawTokenwhich still has a poending state change:Impact
Vulnerable coding pattern
Code Snippet
https://github.com/sherlock-audit/2022-11-bullvbear/blob/main/bvb-protocol/src/BvbProtocol.sol#L450 https://github.com/sherlock-audit/2022-11-bullvbear/blob/main/bvb-protocol/src/BvbProtocol.sol#L521
Tool used
Manual Review
Recommendation
Consider adding nonReentrant to the functions
withdrawTokenandtransferPositionDuplicate of #88