rmrk-team / rmrk-substrate

[WIP] RMRK Substrate pallets
https://rmrk.app
Other
74 stars 36 forks source link

Unequip exploit #185

Closed mrshiposha closed 2 years ago

mrshiposha commented 2 years ago

Anyone can unequip any NFT.

Here is an example:

mrshiposha commented 2 years ago

I believe it is a great idea to decouple equip and unequip as suggested in #147

bmacer commented 2 years ago

I tried to write a test to prove this problem exists (and then gets solved). but have a problem... BOB get's a CollectionNotEquippable error, because ALICE's base doesn't allow equipping by collection 1 (BOBs). Did you make BOB's collection equippable by ALICE's base or did BOB create his own base?

Regardless, I'm working on decoupling equip from unequip, which presumably will solve this, though I was hoping to have a test in place anyway. But abandoning for now (to finish the unequip).

Here's how far I got:

/// Base: Testing decoupling equip and unequip
///
/// [INITIAL STATE BEGIN]
// Alice creates a collection A
// Alice creates a base 0 (baseId = 0) with a slot (slotId = 0)
// Alice mints a parent NFT inside the collection A with a composable resource (baseId = 0)
// Alice mints a child NFT inside the collection A with a slot resource (baseId = 0, slotId = 0)
// Alice sends the child NFT to the parent NFT (all inside the collection A)
// Alice equips the child NFT onto the parent NFT
// Bob creates a collection B
// Bob mints a parent NFT inside the collection B with a composable resource (baseId = 0)
// Bob mints a child NFT inside the collection B with a slot resource (baseId = 0, slotId = 0)
// Bob sends the child NFT to the parent NFT (all inside the collection B)
// Bob equips the child NFT onto the parent NFT
// [INITIAL STATE END]
// Bob tries to equip the child NFT from the collection A (which is root-owned by Alice)
// The Alice's NFT gets unequipped (the equipped flag is reset)
#[test]
fn equip_and_unequip_decoupled() {
    ExtBuilder::default().build().execute_with(|| {
        // Alice creates a collection A
        assert_ok!(RmrkCore::create_collection(
            Origin::signed(ALICE),
            stb("ipfs://col0-metadata"), // metadata
            Some(5),                     // max
            sbvec!["COL0"]               // symbol
        ));

        // // Slot part left hand can equip items from collections 0
        let slot_part = SlotPart {
            id: 0,
            z: 0,
            src: Some(stb("left-hand")),
            equippable: EquippableList::Custom(bvec![
                0, // Collection 0
            ]),
        };

        // Let's create a base with this slot part
        assert_ok!(RmrkEquip::create_base(
            Origin::signed(ALICE), // origin
            stb("svg"),            // base_type
            stb("KANPEOPLE"),      // symbol
            bvec![PartType::SlotPart(slot_part),],
        ));

        // Create Composable resource
        let composable_resource = ComposableResource {
            parts: vec![0].try_into().unwrap(), // BoundedVec of Parts
            src: Some(stbd("ipfs://backup-src")),
            base: 0, // BaseID
            license: None,
            metadata: None,
            thumb: None,
        };
        // Resources to add
        let resources = bvec![ResourceTypes::Composable(composable_resource.clone()),];

        // Mint NFT 0 from collection 0 with
        assert_ok!(RmrkCore::mint_nft(
            Origin::signed(ALICE),
            None,                               // owner
            0,                                  // collection ID
            Some(ALICE),                        // recipient
            Some(Permill::from_float(1.525)),   // royalties
            stb("ipfs://character-0-metadata"), // metadata
            true,
            Some(resources),
        ));

        // Create Slot resource for item (0, 1)
        let slot_resource = SlotResource {
            src: Some(stbd("ipfs://sword-metadata-left")),
            base: 0, // BaseID
            license: None,
            metadata: None,
            slot: 0, // SlotID
            thumb: None,
        };

        // Resources to add
        let resources = bvec![ResourceTypes::Slot(slot_resource.clone()),];

        // Mint NFT (0, 1)
        assert_ok!(RmrkCore::mint_nft(
            Origin::signed(ALICE),
            None,                               // owner
            0,                                  // collection ID
            Some(ALICE),                        // recipient
            Some(Permill::from_float(1.525)),   // royalties
            stb("ipfs://character-0-metadata"), // metadata
            true,
            Some(resources),
        ));

        // ALICE sends (0, 1) to (0, 0)
        assert_ok!(RmrkCore::send(
            Origin::signed(ALICE),
            0,
            1,
            AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0)
        ));

        // ALICE equips (0, 1) to (0, 0)
        assert_ok!(RmrkEquip::equip(Origin::signed(ALICE), (0, 1), (0, 0), 0, 0, 0));

        // BOB creates a collection 1
        assert_ok!(RmrkCore::create_collection(
            Origin::signed(BOB),
            stb("ipfs://col0-metadata"), // metadata
            Some(5),                     // max
            sbvec!["COL0"]               // symbol
        ));

        // Create Composable resource
        let composable_resource = ComposableResource {
            parts: vec![0].try_into().unwrap(), // BoundedVec of Parts
            src: Some(stbd("ipfs://backup-src")),
            base: 0, // BaseID
            license: None,
            metadata: None,
            thumb: None,
        };

        // Resources to add
        let resources = bvec![ResourceTypes::Composable(composable_resource.clone()),];

        // BOB Mints NFT (1, 0) with composable resource referencing ALICE's base 0
        assert_ok!(RmrkCore::mint_nft(
            Origin::signed(BOB),
            None,                               // owner
            1,                                  // collection ID
            Some(BOB),                          // recipient
            Some(Permill::from_float(1.525)),   // royalties
            stb("ipfs://character-0-metadata"), // metadata
            true,
            Some(resources),
        ));

        // Create Slot resource for item (0, 1)
        let slot_resource = SlotResource {
            src: Some(stbd("ipfs://sword-metadata-left")),
            base: 0, // BaseID
            license: None,
            metadata: None,
            slot: 0, // SlotID
            thumb: None,
        };

        // Resources to add
        let resources = bvec![ResourceTypes::Slot(slot_resource.clone()),];

        // BOB Mints NFT (1, 1) with slot resource referencing base 0, slot 0
        assert_ok!(RmrkCore::mint_nft(
            Origin::signed(BOB),
            None,                               // owner
            1,                                  // collection ID
            Some(ALICE),                        // recipient
            Some(Permill::from_float(1.525)),   // royalties
            stb("ipfs://character-0-metadata"), // metadata
            true,
            Some(resources),
        ));

        // BOB sends (1, 1) to (1, 0)
        assert_ok!(RmrkCore::send(
            Origin::signed(BOB),
            1,
            1,
            AccountIdOrCollectionNftTuple::CollectionAndNftTuple(1, 0)
        ));

        // BOB equips (1, 1) to (1, 0)
        assert_ok!(RmrkEquip::equip(Origin::signed(BOB), (1, 1), (1, 0), 0, 0, 0));
    });
}
ilionic commented 2 years ago

Tried to reproduce and got:

system.ExtrinsicFailed
rmrkEquip.ItemAlreadyEquipped

at the following step: