search

code-423n4 / 2021-06-tracer-findings

1 stars 0 forks source link

Lack of enforcement of non-unique marketId’s may confuse/trick users leading to cross-market risk
 #45

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The market creator is untrusted and is expected to provide the _data for Tracer market creation which also includes the marketId which means that there is no enforcement on unique marketId’s.

Impact: Market creators can register any marketId, even clashing with existing markets, and it will be accepted. Even though the market addresses are unique, the same IDs may create confusion among users in front-end UIs (which may track the event emitted with marketID and address) and lead to cross-market risks if users interact with the wrong markets thinking market address was updated for the existing marketID.

Proof of Concept

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualsFactory.sol#L51

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualsFactory.sol#L85

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/deployers/PerpsDeployerV1.sol#L11-L40

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualSwaps.sol#L109

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualsFactory.sol#L111

Tools Used

Manual Analysis

Recommended Mitigation Steps

Enforce assignment of new/unique market IDs in the factory perhaps using the tracerCounter as the ID.

raymogg commented 3 years ago

Market IDs are not used at all throughout the protocol to reference markets. They are simply used to display the name of the market on the frontend.

There is no way to enforce that a market ID matches the actual market. I can call my market TSLA/USD and have it track the ETH/USD price. This is where a more complex market reputation type system will be needed to ensure markets are what they say they are.

As such this is a non issue as it does not affect the protocol. For the initial roll out we will only be running a few markets. Once we open this up to user deployed markets, a system similar to uniswap (warning users this market is user deployed and having community curated lists) will be used.

cemozerr commented 3 years ago

Marking this as invalid as it does not pose any threats to protocols or users (assuming they not build their own front-end).