c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L369

Vulnerability details

Impact

It is recommended to use safeTransferFrom() instead of transferFrom() when transferring ERC721s out of the vault. There is the potential loss of NFTs should the recipient is unable to handle the sent ERC721s.

Proof of Concept

It helps ensure that the recipient is indeed capable of handling ERC721s.

Tools Used

None

Recommended Mitigation Steps

use safeTransferFrom() whenever possible.

IERC721(underlyingContract).transferFrom(address(this), msg.sender, erc721UnderlyingTokenId);
IERC721(underlyingContract).safeTransferFrom(address(this), msg.sender, erc721UnderlyingTokenId);

Assessed type

Token-Transfer

c4-judge commented 11 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 10 months ago

2L

c4-judge commented 10 months ago

GalloDaSballo marked the issue as grade-c

code-423n4 / 2023-09-delegate-findings

Use `safeTransferFrom()` instead of `transferFrom()` for outgoing erc721 transfers #297