Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/SeawaterAMM.sol#L362-L363

Vulnerability details

Vulnerability detail

The transferPositionEEC7A3CD function in the SeawaterAMM contract lacks proper access control. Despite a comment indicating it should only be called by the position manager contract, the function is publicly accessible to any external caller.

// called by the position manager contract!!
/// @inheritdoc ISeawaterExecutorPosition
function transferPositionEEC7A3CD(
    uint256 /* id */,
    address /* from */,
    address /* to */
) external {
    directDelegate(_getExecutorPosition());
}

This function delegates the call to another contract (_getExecutorPosition()), but the access control should be implemented at this entry point.

Impact

The lack of access control on this function could allow unauthorized parties to transfer positions between arbitrary addresses. This could lead to:

Theft of valuable positions from legitimate users
Manipulation of the position management system
Potential financial losses for users
Loss of trust in the overall system integrity

Proof of Concept

function transferPositionEEC7A3CD(
    uint256 /* id */,
    address /* from */,
    address /* to */
) external {
    directDelegate(_getExecutorPosition());
}

An attacker could simply call this function with any parameters, potentially transferring positions they don't own:

// Assuming SeawaterAMM is deployed at 0x123...
SeawaterAMM amm = SeawaterAMM(0x123...);
amm.transferPositionEEC7A3CD(positionId, victimAddress, attackerAddress);

Tools Used

Manual code review

Recommended Mitigation Steps

Implement a modifier to check the caller and apply it to the function:

   modifier onlyPositionManager() {
       require(msg.sender == positionManager, "Caller is not the position manager");
       _;
   }

Assessed type

Access Control

code-423n4 / 2024-08-superposition-validation

Unprotected Position Transfer Function #192