code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Upgraded Q -> 2 from #255 [1684436602164] #508

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #255 as 2 risk. The relevant finding follows:

QA-03: Anyone can memorialize LP positions from another user Description The function PositionManager.memorializePositions contains no access control. This means anyone can memorialize other LP's positions, provided that they have approved the PositionManager to transfer their LPs.

function memorializePositions(
    MemorializePositionsParams calldata params_
) external override {
    EnumerableSet.UintSet storage positionIndex = positionIndexes[params_.tokenId];

    IPool   pool  = IPool(poolKey[params_.tokenId]);
    // @audit-ok anyone can memorialize on behalf of owner who minted the NFT & approved to PositionManager
    address owner = ownerOf(params_.tokenId);

Since the PositionManager will be a "singleton" contract, it is expected that this approval will be set to true for most users. This opens door for any users memorializing positions even if the owner does not want it.

Recommendation This may be a design decision, but it is recommended that only the owner or the NFT operator are able to manage positions in any way, such as calling PositionsManager.memorializePositions.

See the markdown file with the details of this report here.

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #356

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #488

c4-judge commented 1 year ago

Picodes marked the issue as partial-50

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes changed the severity to 3 (High Risk)