code-423n4 / 2021-09-sushitrident-2-findings

0 stars 0 forks source link

`subscribe` can be called by anyone #77

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xsanson

Vulnerability details

Impact

Function subscribe in ConcentratedLiquidityPoolManager lets anyone call it for another user's position. This is probably unintentional, since in claimReward the check require(ownerOf[positionId] == msg.sender, "OWNER") is present.

The issue is that an user gets subscribed to multiple spam incentives that, when listed on UI, become annoying.

Proof of Concept

https://github.com/sushiswap/trident/blob/c405f3402a1ed336244053f8186742d2da5975e9/contracts/pool/concentrated/ConcentratedLiquidityPoolManager.sol#L65

Tools Used

editor

Recommended Mitigation Steps

Consider adding a require(ownerOf[positionId] == msg.sender, "OWNER").

sarangparikh22 commented 3 years ago

Duplicate of #69 It's an non-critical issue, as it doesn't effect the pool in any way. Disagree with severity, it should be 0.

alcueca commented 2 years ago

Assets not at risk, but function incorrect as to spec. You can't argue that this is an intended behaviour. Severity 1 is correct.

sarangparikh22 commented 2 years ago

@alcueca During the time of the audit, we were still finishing the spec, at that time we had this subscribe function open on a recommendation by another team member, however after internal discussion we decided it to make it only callable by the respective owner of the position only. This makes this issue valid.