Centrifuge - jkoppel's results

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/0af232255c7d045efde4ac40801dfeeed8a8d889/src/LiquidityPool.sol#L98

Vulnerability details

Impact

withApproval in LiquidityManager.sol has the documentation

   /// @dev Either msg.sender is the owner or a ward on the contract

However, the code makes it very clear that only owners have approval, never wards.

    modifier withApproval(address owner) {
        require(msg.sender == owner, "LiquidityPool/no-approval");
        _;
    }

The upshot is that only owners of positions can modify them. Wards cannot, even though the documentation makes it clear they should be able to.

Proof of Concept

I modified testDepositWithApproval in Liquidity.t.sol to try the deposit where the test contract is a ward.

        root.relyContract(lPool_, self); // give self auth permissions
        lPool.requestDeposit(amount, address(investor));

However, the transaction reverted with "LiquidityPool/no-approval", even though the caller is a ward and should have this power according to the docs.

Tools Used

Manuel inspection

Recommended Mitigation Steps

    modifier withApproval(address owner) {
        require(msg.sender == owner || wards[msg.sender] == 1, "LiquidityPool/no-approval");
        _;
    }

Assessed type

Access Control