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

[Bug]: Marmalade-v2 uri updates with no restriction by default #200

Open
daplcor opened this issue May 8, 2024 · 0 comments
Open

[Bug]: Marmalade-v2 uri updates with no restriction by default #200

daplcor opened this issue May 8, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@daplcor
Copy link
Contributor

daplcor commented May 8, 2024

Expected Behavior

By default NFT’s are protected from malicious attempts to alter or edit in any way. This is one of the cornerstones of blockchain technology and we’ve seen the result before with principled accounts where guards were altered. There were recently a number of changes to Marmalade-v2 to produce v2.2 essentially. One of these changes introduced the ability to update an NFT’s URI.

There are definitely good use cases for this, but by default when you mint, anyone can change the URI as shown in the example below. There are extra steps you can take to disable this behavior, but that should be default out of the box to avoid issues of the past like hijacking. The argument that all policies are by default true, therefore uri should be true is false as demonstrated by trying to burn and sell an NFT.

I have no opposition to including new features, but we should be very careful when altering the default mint process. If people aren’t careful, it takes only a second to modify an NFT that doesn’t explicitly deny being able to change or restrict the feature. One approach could simply be to add an enforce like (enforce-valid-uri-account id account) which would put it in line with our other functions.

One additional conversation involves this default as an NFT should be minted with no such mechanism to update URI, to me that sounds like a separate policy altogether. In marmalde-v1 for example we went the full nine yards to protect and prove uri/metadata via hashing mechanisms. Unless I missed somewhere, how does a user or marketplace quickly query if this feature is enabled or disabled? From indexing to presenting NFTs on a marketplace it presents unique challenges, which may prevent those NFT’s from being considered for listing.

Current Behavior

When you mint a default policy NFT, it allows anyone to change the URI without restriction.

Possible Solution

enforce the nft owner or specified guard if one is chosen.

If I read the guard-policy-v1 it says by default it should be guard_failure, but the if statement only evaluates v0 nfts. It looks like the following function might just need to evaluate if v0 and then evaluate if v1 and no guard set then fail else validate guard.

(defun get-uri-guard:guard (token-id:string)
(let ((version:integer (marmalade-v2.ledger.get-version token-id)))
(if (= version 0)
GUARD_FAILURE
(with-read uri-guards token-id
{
"uri-guard":= uri-guard
}
uri-guard
)
)
)
)

Steps to Reproduce

All TX come from https://explorer.chainweb.com/mainnet

Mint with DEFAULT policy, nothing special
TX: 8R1X9595hCk9Sv2z0kHYx4-6cLHCVR7BIEzgpY3JBUI

Burn is prevented by default even by the creator.
TX: XSP_OV6uvUwUd8l3G2-jCYUvKQkjaF7P0g1XL70lpkU

URI is allowed to be changed by default by anyone
TX: Du6BtthJuhZy5QVL2C-VVEJBREUvPEQD2D7J7eSqlwk

Listing NFT for sale by default fails unless it’s done by the owner account
TX: yL0EEhVnWXmRnZvnfSA9RBqwXf_izYZ4VnA6mhyln2g

Transfer an NFT by default fails when the owner keyset doesn’t sign
TX: sOMsNLmcBw47aHUbOgSMeI3w2FSOZALzgUl4wHNEGhs

Relevant log output

No response

@daplcor daplcor added the bug Something isn't working label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant