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.

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

Reentrancy attacks in mint function #489

Closed
code423n4 opened this issue Dec 16, 2022 · 2 comments
Closed

Reentrancy attacks in mint function #489

code423n4 opened this issue Dec 16, 2022 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-400 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Position.sol#L126-L161
https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L156-L210
https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L307-L349

Vulnerability details

Author: rotcivegaf

Impact

The root of the problem are in the mint who use _safeMint. This mint call the function onERC721Received of the token recipient, generating a possible reentrancy attack

Proof of Concept

A wallet can call the initiateMarketOrder or initiateLimitOrder and this functions call the mint function who inside call the OZ _safeMint
When the the _safeMint call onERC721Received of the _mintTrade.account, this one have the possibility to call functions and the lines L149-L160 of mint are not execute yet, also L207-L210 of initiateMarketOrder and L347-L348 of initiateLimitOrder
The _mintTrade.account can call:

Tools Used

Review

Recommended Mitigation Steps

Can use reentrancy guards in the three function mint, initiateMarketOrder and initiateLimitOrder
Or use _mint instead of _safeMint

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 16, 2022
code423n4 added a commit that referenced this issue Dec 16, 2022
@code423n4 code423n4 changed the title Possibly reentrancy attacks in mint function Reentrancy attacks in mint function Dec 16, 2022
@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as duplicate of #400

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-400 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants