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

2 stars 1 forks source link

Unchecked token transfer #321

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenTransferHelpers.sol#L18

Vulnerability details

Impact

Unchecked token transfer

Proof of Concept

Since the contract will work with many different ERC721 tokens, and not all of them are based, for instance, on the OZ ERC721 token contract. And some tokens can return False instead of reverting a transaction in case of any failed/errors while the transfer

This can lead to the contract will think that the token transfer was successful when it is not.

It could lead to a similar issue as this one - https://twitter.com/slowmist_team/status/1700782725593268448

Tools Used

Manual review

Recommended Mitigation Steps

Use OZ SafeTransfer Library

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

Assessed type

Invalid Validation

c4-judge commented 11 months ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

GalloDaSballo commented 11 months ago

ERC721 has no return value