[MARKETPLACE] Investigate weird signing issue

bitbeckers commented 6 months ago

Which area(s) of Hypercerts are affected? (leave empty if unsure)

No response

To Reproduce

Look into siging issue where a 3rd user can execute for user 1's ask, with user 2's bid.

Describe the Bug

.

Expected Behavior

.

Jipperism commented 6 months ago

The initial message https://discord.com/channels/1140327804446183444/1163506245668458496/1218217688355180564

Jipperism commented 6 months ago

Steps to reproduce:

Create fraction belonging to address A
Sign a maker order using address B (expected to succeed, as simply signing the order is not something we are checking in the marketplace SDK. Possibly we can start checking for this, but it would be a UX improvement as we cannot prevent people from writing code that would allow them to sign the order using a different order themselves. The UI has been build to prevent this already)
Register order with API. (expected to fail: this should not be possible as we don't want invalid orders to be registered with the API, or showing up in the marketplace UI. This ).
Perform sale by calling contract with invalidly signed maker order (expected to fail, on a contract level).

Jipperism commented 6 months ago

@bitbeckers I'm failing to reproduce this.

I've bypassed a number of failsafes by running locally and created a makerorder for a token owned by address A, signed by address B, and registered it with our backend.

Fractional sale: It fails with a HypercertSplitFractionError for a fractional sale. See Tenderly transaction. Example page with wrongly signed order.

Direct sale, with splitting upon order creation: errors out with HypercertSplitFractionError as well, as you can't split a fraction that isn't yours.

Direct sale, bypassing splitting by putting up a fraction directly: It fails in my browser with ERC1155SafeTransferFromFail. See Tenderly simulation. Example of wrongly signed order.

Deployment that allows for creating and registering invalidly signed orders.

Would you agree we can close this for now, or do you want to investigate further?

Jipperism commented 6 months ago

Diving into this transaction, as it was implied this was faulty

Claimed to: this tx was purchasing of an order that created by owner of the claim, not created by claimToken owner

Buyer: 0x42FbF4d890B4EFA0FB0b56a9Cc2c5EB0e07C1536 Seller: 0x8B014474b731BdCdF42ED35719FA37A638B33d97 Owner of claimToken that got sold:0x925b8a9562d57712e5efa6ce8bcc5dd8a9afe2af. It's a Gnosis Safe. ClaimId: 11155111-0xa16dfb32eb140a6f3f2ac68f41dad8c7e83c4941-83369179895629923548526778820783211806720

Query

{
  claim(
    id: "11155111-0xa16dfb32eb140a6f3f2ac68f41dad8c7e83c4941-83369179895629923548526778820783211806720"
  ) {
    id
    owner
    creator
  }
  claimTokens(where: {tokenID_in: ["83369179895629923548526778820783211806721", "83369179895629923548526778820783211806722"]}) {
    id
    owner
    units

  }
}

gives

{
  "data": {
    "claim": {
      "id": "11155111-0xa16dfb32eb140a6f3f2ac68f41dad8c7e83c4941-83369179895629923548526778820783211806720",
      "owner": "0x8b014474b731bdcdf42ed35719fa37a638b33d97",
      "creator": "0x8b014474b731bdcdf42ed35719fa37a638b33d97"
    },
    "claimTokens": [
      {
        "id": "11155111-0xa16dfb32eb140a6f3f2ac68f41dad8c7e83c4941-83369179895629923548526778820783211806721",
        "owner": "0x925b8a9562d57712e5efa6ce8bcc5dd8a9afe2af",
        "units": "999999999999999999"
      },
      {
        "id": "11155111-0xa16dfb32eb140a6f3f2ac68f41dad8c7e83c4941-83369179895629923548526778820783211806722",
        "owner": "0x42fbf4d890b4efa0fb0b56a9cc2c5eb0e07c1536",
        "units": "1"
      }
    ]
  }
}

Order from the DB:

id	createdAt	quoteType	globalNonce	orderNonce	strategyId	collectionType	collection	currency	signer	startTime	endTime	price	signature	additionalParameters	chainId	subsetNonce	itemIds	amounts
e504e5f4-d889-4e1e-a326-d71107389c64	2024-03-15 14:51:15.038799+00	1	0	3	10	2	0xa16DFb32Eb140a6f3F2AC68f41dAd8c7e83C4941	0x7b79995e5f793A07Bc00c21412e50Ecae098E7f9	0x8B014474b731BdCdF42ED35719FA37A638B33d97	1710514270	1719154270	1000000000000	0xe6fe5f034b861dbb5f12a321a431b94df59fdebb2e57aeea84553df2d470b4a8013808795f32bbedd45067d0ada59b3ef4d89ceb5ed1b708ab276da9bcb5a9511b	0x0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000003b9aca0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001	11155111	0	["83369179895629923548526778820783211806721"]	["1"]

Gnosis safe logs: (at the bottom is the token being transfered to the safe address)

Reconstructing timeline:

??? -> Seller: 0x8B014474b731BdCdF42ED35719FA37A638B33d97 obtained claims
Mar-13-2024 09:34:00 PM +UTC -> Seller does multi send to transfer claims to safe.
2024-03-15 14:51:15.038799+00 -> Seller creates order to sell tokens that he does not own anymore. This still works because API side check, to verify if user owns all claimTokens has not been implemented yet. Either way, the transaction could be stored but should not resolve.
Mar-15-2024 03:11:12 PM +UTC -> [Buyer is able to complete order]((https://sepolia.etherscan.io/tx/0x12daf2eef5521336565bab47905e57fddead453c49a50a4a06265e77566cef55)

bitbeckers commented 5 months ago

FIxed and reported in #1320

hypercerts-org / hypercerts