search

hicetnunc2000 / objkt-swap

Hic et Nunc smart contracts. FA2 multiassets: hDAO, OBJKTs, Marketplace, SUBJKTs and Unregistry.
79 stars 48 forks source link

v1 post mortem and v2 migration #2

Open crzypatchwork opened 3 years ago

crzypatchwork commented 3 years ago

"They have already served, indeed have already been overcome." -- Brecht

Due an exploit in our marketplace smart contract we've been currently working torwards a contract migration. A detailed report can be found at: https://docs.google.com/document/d/1eZgAotkL3s0M9Dc3RahDWcXarFH0dR9_CMzQsvU6mYk Attackers would take possession of assets under the marketplace custody due such exploit.

We've updated the manager of the main smart contract and inactivated all listings as an immediate response (https://better-call.dev/mainnet/opg/opaD3kRPZ8SAB9F24msb68LnvgFpaDL9cx4Y7nguaofKCBaoomh), which would diminish such exploit as far as we know. Such response was taken in the best habil time we could have handled, being the only way found to stop the vulnerability across the whole protocol. We're sorry about such event, and we are open for discussing solutions for those who were affected.

We hope to keep optimizing the protocol, presenting a new set of smart contracts possibly by early July. We also have other challenges that just the improvement of a protocol might not be able to solve. It's each one's responsibility though to remain true to it's own values. Minting is still available even though it's subject to changes.

To better protect your assets and to assist on the migration we ask one to visit the 'v1 swaps' tab under your profile. https://hicetnunc.xyz/tz/<address>/v1

auditing tools can be found at: https://www.hictory.xyz/ https://nftbiker.xyz/

mazlow17 commented 3 years ago

I had this in discord but I am posting it again here.

I would recommend that the new contract's first step when called, validate the original sender by tezos prefix, if the originator prefix is a wallet the interaction can pass to the next step, if the originator has a "KT" or contract prefix then it must match a validation criteria. The new contract should contain store values for "admin" privileged contracts that will have the KT prefix but be allowed to proceed with interactions. The interaction of admin changes are managed by a super user wallet that can interact exclusively with the adding/removing admin contract flags. To my understanding quipuswap uses this kind of safety check or something similar. This should significantly help prevent this bug and future bugs that utilize advanced interaction functionalities when a transaction originates from a contract. If developers want to make there own tools like objkt.bid, they can be added to the white list by the team here after being vetted, and if this policy/contract would need to be change again in a later update to support things like split royalties, that can be done carefully rather than in the wake of a hack.

NoRulesJustFeels commented 3 years ago

I've created this version of the postmortem (comments accepted): https://docs.google.com/document/d/1eZgAotkL3s0M9Dc3RahDWcXarFH0dR9_CMzQsvU6mYk/edit#