code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol#L150

Vulnerability details

Impact

When confirming the renouncement of ownership, there are no LSP1 hooks, such as sending a _TYPEID_LSP14_OwnershipTransferred_SenderNotification message.

As a result, contracts that use LSP10 data keys will contain keys for addresses for which they are no longer the owner of, contradicting the LSP10 standard.

Proof of Concept

A test written in foundry is provided where a universal profile implementing an LSP1 UniversalReceiver delegation still has keys for vaults that have been renounced.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.4;

import "forge-std/Test.sol";
import "../src/UniversalProfile.sol";
import "../src/LSP9Vault/LSP9Vault.sol";
import "../src/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol";
import "../src/Mocks/LSP20Owners/FallbackReturnMagicValue.sol";

import {BytesLib} from "solidity-bytes-utils/contracts/BytesLib.sol";

import {_LSP1_UNIVERSAL_RECEIVER_DELEGATE_KEY} from "../src/LSP1UniversalReceiver/LSP1Constants.sol";
import {_LSP10_VAULTS_ARRAY_KEY} from "../src/LSP10ReceivedVaults/LSP10Constants.sol";

contract LSP14Test is Test {
    using BytesLib for bytes;

    UniversalProfile profile;
    LSP9Vault vault;
    LSP1UniversalReceiverDelegateUP LSP1Delegate;
    FallbackReturnMagicValue LSP20Owner;

    function setUp() public {
        LSP20Owner = new FallbackReturnMagicValue();
        profile = new UniversalProfile(address(LSP20Owner));
        LSP1Delegate = new LSP1UniversalReceiverDelegateUP();
        profile.setData(_LSP1_UNIVERSAL_RECEIVER_DELEGATE_KEY, bytes.concat(bytes20(address(LSP1Delegate))));
    }

    function testLSP9DataKeyWhenNotOwner() public {
        // Check LSP10 array length is 0
        bytes memory arrayLength = profile.getData(_LSP10_VAULTS_ARRAY_KEY);
        assert(arrayLength.length == 0);

        // Create a vault and acquire vault keys
        vault = new LSP9Vault(address(profile));
        arrayLength = profile.getData(_LSP10_VAULTS_ARRAY_KEY);
        assert(uint128(bytes16(arrayLength)) == 1);
        assert(vault.owner() == address(profile));

        // Renounce ownership of vault
        vm.startPrank(address(profile));
        vault.renounceOwnership();
        vm.roll(block.number + 200);
        vault.renounceOwnership();
        vm.stopPrank();

        // Check owner of vault is 0, but array length is 1
        assert(vault.owner() == address(0));
        arrayLength = profile.getData(_LSP10_VAULTS_ARRAY_KEY);
        assert(uint128(bytes16(arrayLength)) == 1);
    }
}