code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/BondNFT.sol#L71 https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Lock.sol#L75

Vulnerability details

Impact

Wrong address input in createLock results in function failure of extendLock and claim.

Proof of Concept

When someone calls lock in Lock.sol, the function BondNFT.createLock is called .

    bondNFT.createLock( _asset, _amount, _period, msg.sender);

bondNFT.createLock takes in 4 input parameters, _asset, _amount, _period and msg.sender.

function createLock(
    address _asset,
    uint _amount,
    uint _period,
    address _owner
) external onlyManager() returns(uint id) {

When creating the lock and bond, the address is incorrectly written as address(0).

        Bond memory _bond = Bond(
            id,             // id
            address(0),     // owner

In extendLock, bond.owner == _sender is checked. If bond.owner is address(0) and _sender is the address extending the bond (not address(0)), then extendLock cannot function.

    require(bond.owner == _sender, "!owner");

Similarly, for claim, if _claimer is not bond.owner, then claim cannot be processed.

Tools Used

Manual Review

Recommended Mitigation Steps

The owner should not be address(0) but msg.sender instead. In this case, since msg.sender is passed to createLock, the input parameter _owner should be used instead

        Bond memory _bond = Bond(
            id,             // id
            _owner,     // owner

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #457