Open code423n4 opened 3 years ago
Disputing just as while this is important, its quite explicitly stated in the todo comment and as such is already known by the team as a potential issue.
Realistically shouldn't be too much of a problem with whitelisting of the trader.
Marking this as medium risk as, regardless of being noted by the team, still poses a security threat.
Duplicate of #72
removing duplicate label as per judge
changing risk from 1 to 2 as per judges sheet
Handle
shw
Vulnerability details
Impact
As written in the to-do comments, reentrancy could happen in the
executeTrade
function ofTrader
since themakeOrder.market
can be a user-controlled external contract.Proof of Concept
Referenced code: Trader.sol#L121-L126
Recommended Mitigation Steps
Add a reentrancy guard (e.g., the implementation from OpenZeppelin) to prevent the users from reentering critical functions.