code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP1UniversalReceiver/LSP1UniversalReceiverDelegateUP/LSP1UniversalReceiverDelegateUP.sol#L152

Vulnerability details

Impact

When receiving and sending assets, checks are done so that data keys correspond to digital assets with a positive balance.

When receiving:

            if (notifier.code.length > 0) {
                // if the amount sent is 0, then do not update the keys
                uint256 balance = ILSP7DigitalAsset(notifier).balanceOf(
                    msg.sender
                );
                if (balance == 0) return "LSP1: balance not updated";
            }

When sending:

            // if the amount sent is not the full balance, then do not update the keys
            uint256 balance = ILSP7DigitalAsset(notifier).balanceOf(msg.sender);
            if (balance != 0) return "LSP1: full balance is not sent";

However, it is possible to bypass the check during receiving tokens by transferring a 0 amount in a contract's constructor.

Proof of Concept

The following test written in foundry shows that it is possible to have data keys corresponding to digital assets with a balance of 0.

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

import "forge-std/Test.sol";
import "../src/UniversalProfile.sol";
import "../src/LSP7DigitalAsset/LSP7DigitalAsset.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 {_LSP5_RECEIVED_ASSETS_ARRAY_KEY} from "../src/LSP5ReceivedAssets/LSP5Constants.sol";

contract MockLSP7 is LSP7DigitalAsset {
    constructor(
        string memory name,
        string memory symbol,
        address newOwner,
        uint256 mintAmount
    ) LSP7DigitalAsset(name, symbol, newOwner, false) {
        _mint(newOwner, mintAmount, false, "");
    }
}

contract LSP1Test is Test {
    using BytesLib for bytes;

    UniversalProfile profile;
    MockLSP7 digitalAsset;
    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 testLSP7DataKeyWithZeroBalance() public {
        // Check starting LSP5 array length is 0
        bytes memory arrayLength = profile.getData(_LSP5_RECEIVED_ASSETS_ARRAY_KEY);
        assert(arrayLength.length == 0);

        // Transfer 0 to profile
        digitalAsset = new MockLSP7("Test", "TEST", address(profile), 0);

        // Check new array length is 1 but balance is 0
        arrayLength = profile.getData(_LSP5_RECEIVED_ASSETS_ARRAY_KEY);
        assert(uint128(bytes16(arrayLength)) == 1);
        assert(digitalAsset.balanceOf(address(profile)) == 0);
    }
}