The user's collateral is not held in the market contract but is instead held in individual escrows. Every user has a unique escrow for every market. And the escrow contracts are created via the Market contract's createEscrow() function. And it's initialized with the collateral and user parameters. However, the initialize function of the Escrow is public and there are no modifiers. It can be frontrun by an actor who monitors the mempool for the createEscrow func-sig. Hence, the actor steals the collateral assets of the user.
Proof of Concept
A market wants to deposit its collateral and calls the Market contract's deposit function;
Lines of code
https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L278-L285 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L245-L251 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L246-L248 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/escrows/SimpleERC20Escrow.sol#L25-L29
Vulnerability details
Impact
The user's collateral is not held in the market contract but is instead held in individual escrows. Every user has a unique escrow for every market. And the escrow contracts are created via the Market contract's
createEscrow()
function. And it's initialized with thecollateral
anduser
parameters. However, the initialize function of the Escrow is public and there are no modifiers. It can be frontrun by an actor who monitors the mempool for thecreateEscrow
func-sig. Hence, the actor steals the collateral assets of the user.Proof of Concept
A market wants to deposit its collateral and calls the Market contract's
deposit
function;Permalink
The function checks whether the market has an escrow contract via
getEscrow
internally;Permalink
It's the first time for the market so it returns with creating escrow via
createEscrow
and initializes the escrow via;Permalink
The actor sees the TX in mempool and frontruns the initialize function as it's public and has no modifier.
Permalink
Tools Used
Manual Review
Recommended Mitigation Steps
It's recommended to add an initializer modifier to the initialize function.