search

code-423n4 / 2024-06-krystal-defi-findings

0 stars 0 forks source link

`_swapAndMint()` does not use `safeTransferFrom` #22

Closed c4-bot-9 closed 2 months ago

c4-bot-9 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/Common.sol#L392

Vulnerability details

Impact

Unsafe transfer of ERC721 tokens in _swapAndMint().

Proof of Concept

_swapAndMint() claims to be using safeTransferFrom, but then only uses transferFrom. This is an issue especially in V3Utils where the user might rely on compliance with this safety check on the instructions.recipient as _swapAndMint() is called during the V3Utils.execute() in V3Utils.onERC721Received()

Recommended Mitigation Steps

Use safeTransferFrom() on Common.sol#L392.

Assessed type

ERC721

3docSec commented 2 months ago

Provisionally marking satisfactory for sponsor review

c4-judge commented 2 months ago

3docSec marked the issue as satisfactory

c4-judge commented 2 months ago

3docSec marked the issue as selected for report

Haupc commented 2 months ago

this not effect transferring NFT to recipient

3docSec commented 2 months ago

Marking as insufficient proof:

the user might rely on compliance with this safety check on the instructions.recipient

the user is assumed to enter correct instructions.

As a side note, even though this particular instance at L392 was not reported, this finding has been reported by the 4naly3er report.

c4-judge commented 2 months ago

3docSec marked the issue as unsatisfactory: Insufficient proof