code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

NFT token id repeated #118

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

MaCree

Vulnerability details

Impact

  1. merge funtion may lead to create repeated NFT token id, so user can not lock XDEFI

Proof of Concept

run the test case below please

beforeEach(async () => { [god, account1, account2, account3] = await ethers.getSigners();

    XDEFI = await (await (await ethers.getContractFactory("XDEFI")).deploy("XDEFI", "XDEFI", totalSupply)).deployed();
    XDEFIDistribution = await (await (await ethers.getContractFactory("XDEFIDistribution")).deploy(XDEFI.address, "https://www.xdefi.io/nfts/", 0)).deployed();

    // Setup some bonus multipliers (0 days with 1x, 1 day with 1.2x, 2 days with 1.5x)
    await (await XDEFIDistribution.setLockPeriods([0, 86400, 172800], [100, 120, 150])).wait();

    // Give each account 100 XDEFI
    // change the initial token 
    await (await XDEFI.transfer(account1.address, toWei(3000))).wait();
    await (await XDEFI.transfer(account2.address, toWei(2000))).wait();
    await (await XDEFI.transfer(account3.address, toWei(2000))).wait();
});

it("merge creates repeated token id ", async () => {

    //await hre.ethers.provider.send('evm_increaseTime', [86400]);

    // Position 1 locks, address 1
    const pointsOfPosition1 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account1).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account1).lock(toWei(1000), 0, account1.address)).wait();
    const nft1 = (await XDEFIDistribution.tokenOfOwnerByIndex(account1.address, 0)).toString();
    expect(await XDEFIDistribution.pointsOf(nft1)).to.equal(pointsOfPosition1);

    // Position 2 locks, address 2
    const pointsOfPosition2 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account2).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account2).lock(toWei(1000), 0, account2.address)).wait();
    const nft2 = (await XDEFIDistribution.tokenOfOwnerByIndex(account2.address, 0)).toString();
    expect(await XDEFIDistribution.pointsOf(nft2)).to.equal(pointsOfPosition2);

    // Position 3 locks, address 1
    const pointsOfPosition3 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account1).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account1).lock(toWei(1000), 0, account1.address)).wait();
    const nft3 = (await XDEFIDistribution.tokenOfOwnerByIndex(account1.address, 1)).toString();
    expect(await XDEFIDistribution.pointsOf(nft3)).to.equal(pointsOfPosition3);

    // Position 4 locks, address 3
    const pointsOfPosition4 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account3).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account3).lock(toWei(1000), 0, account3.address)).wait();
    const nft4 = (await XDEFIDistribution.tokenOfOwnerByIndex(account3.address, 0)).toString();
    expect(await XDEFIDistribution.pointsOf(nft4)).to.equal(pointsOfPosition4);

    // Position 5 locks, address 3
    const pointsOfPosition5 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account3).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account3).lock(toWei(1000), 0, account3.address)).wait();
    const nft5 = (await XDEFIDistribution.tokenOfOwnerByIndex(account3.address, 1)).toString();
    expect(await XDEFIDistribution.pointsOf(nft5)).to.equal(pointsOfPosition5);

    // Position 6 locks, address 2
    const pointsOfPosition6 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account2).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account2).lock(toWei(1000), 0, account2.address)).wait();
    const nft6 = (await XDEFIDistribution.tokenOfOwnerByIndex(account2.address, 1)).toString();
    expect(await XDEFIDistribution.pointsOf(nft6)).to.equal(pointsOfPosition6);

    await hre.ethers.provider.send('evm_increaseTime', [172800]);

    //batch unlocked
    await (await XDEFIDistribution.connect(account3).unlockBatch([nft4, nft5], account3.address)).wait();
    await (await XDEFIDistribution.connect(account2).unlockBatch([nft2, nft6], account2.address)).wait();

    //Unlocked positions 4 and 5 are merged into unlocked position 7
    await (await XDEFIDistribution.connect(account3).merge([nft4, nft5], account3.address)).wait();
    expect(await XDEFIDistribution.balanceOf(account3.address)).to.equal('1');
    const nft7 = (await XDEFIDistribution.tokenOfOwnerByIndex(account3.address, 0)).toString();
    // nft7=5

    await (await XDEFIDistribution.connect(account2).merge([nft2, nft6], account2.address)).wait();
    expect(await XDEFIDistribution.balanceOf(account2.address)).to.equal('1');
    const nft8 = (await XDEFIDistribution.tokenOfOwnerByIndex(account2.address, 0)).toString();
    // nft8=4

    //Position 9  locks, address 1
    const pointsOfPosition9 = (await XDEFIDistribution.getPoints(toWei(1000), 0)).toString();
    await (await XDEFI.connect(account1).approve(XDEFIDistribution.address, toWei(1000))).wait();
    await (await XDEFIDistribution.connect(account1).lock(toWei(1000), 0, account1.address)).wait();
    const nft9 = (await XDEFIDistribution.tokenOfOwnerByIndex(account1.address, 0)).toString();
    expect(await XDEFIDistribution.pointsOf(nft1)).to.equal(pointsOfPosition9);
    // nft9=5

    //duplicate token id 
    expect(nft7).to.equal(nft9);
});

Tools Used

XDEFIDistribution.js test cases

Recommended Mitigation Steps

setLockPeriods() not using duration 0 may avoid this bug(? maybe not a bug)

deluca-mike commented 2 years ago

Valid and duplicate of #17