Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unequip exploit #185

Closed
mrshiposha opened this issue Jun 21, 2022 · 3 comments
Closed

Unequip exploit #185

mrshiposha opened this issue Jun 21, 2022 · 3 comments

Comments

@mrshiposha
Copy link
Contributor

Anyone can unequip any NFT.

Here is an example:

  • [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)
@mrshiposha
Copy link
Contributor Author

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

@bmacer
Copy link
Contributor

bmacer commented Jun 25, 2022

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
Copy link
Contributor

ilionic commented Jul 28, 2022

Tried to reproduce and got:

system.ExtrinsicFailed
rmrkEquip.ItemAlreadyEquipped

at the following step:

  • Bob tries to equip the child NFT from the collection A (which is root-owned by Alice)

@ilionic ilionic closed this as completed Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants