Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

When the burnToMint function is enabled, any bad actor can transfer the nft-to-burn when it receives the newly minted nft, getting burned after he no longer is the owner. #1988

Closed
c4-submissions opened this issue Nov 13, 2023 · 6 comments
Closed

When the burnToMint function is enabled, any bad actor can transfer the nft-to-burn when it receives the newly minted nft, getting burned after he no longer is the owner. #1988

c4-submissions opened this issue Nov 13, 2023 · 6 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-1597 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L220

Vulnerability details

Impact

Medium impact, it affects the functionallity of the nfts with other contracts, as they wouldn't want to interact with tokens that could get burned.

Explanation

in the NextGenCore contract, the burnToMint function has the minting of the new nft, which makes an external call via _safeMint, before the burning of the nft-to-burn. This allows any bad actor to transfer the nft to a victim contract without the victim contract being able to stop the _burn from happening.

Recommended Mitigation Steps

Make the mint of the nft in burnToMint happen after the burning of the nft that gives access to the mint.

Assessed type

Token-Transfer
@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 13, 2023
c4-submissions added a commit that referenced this issue Nov 13, 2023
@c4-submissions
CaeraDenoir issue #1988
6aad5f2
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1198

@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1597

@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1742

@c4-judge c4-judge reopened this Nov 29, 2023
@c4-judge
Copy link

alex-ppg marked the issue as not a duplicate

@c4-judge
Copy link

alex-ppg marked the issue as duplicate of #1597

@c4-judge c4-judge added the partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) label Dec 8, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-1597 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@c4-judge @c4-pre-sort @c4-submissions