Trustline flag set on NFTokenMint TX's Can Max a Minters Reserve and Create Infinite Issued Currencies with No Reserve (Version: 1.9.2)

[x-Tokenize]

---

Name: Trustline flag set on NFTokenMint TX's Can Max a Minters Reserve and Create Infinite Issued Currencies with No Reserve

About:

NFTs Utilizing Flag 12/13 on NFTokenMint Tx's with a Transfer Fee >0 are susceptible to spam trust lines and an increase in their reserve requirement while also allowing a currency issuer to create infinite currencies for no cost or reserve requirement.

labels:

NFTokenMint, Trustline Spam, neglibile reserve requirement

---

Issue Description

NFTs minted with the trustline & transferable flag enabled with a TransferFee > 0 are susceptible to attack by a malicious user which allows the attacker to create an infinite number of currencies on their attacking issuing account and maxes out the victims reserve requirement on the NFT account. At conception, this attack was very expensive to execute as an attacker but after optimization in the attacking method, it became a negligible cost and allowed the Currency issuer to issue assets without affecting their reserve requirement.

Steps to Reproduce

To reproduce, there are 4 accounts involved in the attack:

Victim Account: Issues an NFT, that is transferable, auto-creates the trustline and has a transfer fee > 0\ Attacking Account A: Will be setting trustlines, creating and accepting NFT offers and sending back token to issuer. Attacking Account B: Same as Account A Attacking Currency Issuer (Account C): Issues Currencies to Account A and Account B

Attack sequence: 1) Account A acquires a vulnerable NFT 2) Account B creates a trustline to Account C for random currency 'xyz' 3) Account C sends 1 'xyz' to Account B 4) Account A creates a sell offer for NFT with destination set to Account B for 1 'xyz' 5) Account B Accepts the sell offer from Account A (Account A Gets (1 'xyz'- (transferFee1 'xyz')) and Victim gets (transferFee 1 'xyz')) 6) Account B removes the trustline for 'xyz' 7) Account A Sends the received amount back to the issuer (this results in clearing reserve requirement for Account C) 8) Repeat the process starting at step 2 with reversing the roles of Account A and Account B

Expected Result

The Attacker would be able to clog up the reserve requirement of the NFT issuer coming with a high cost to the attacker.

Actual Result

With the reserve requirement also becoming negligible, it allowed Account C to issue an infinite amount of currencies and max out the vulnerable NFT Minters reserve with a negligible cost to the attacker (only transaction fees).

Environment

Tested on rippled 1.9.2

Supporting

https://xls20.bithomp.com/explorer/r9LFTf81MW7ig1gXHcbaBKPh117szuf3XE

[alloynetworks]

Having read this both here and in one on one discussions with the OP, I am inclined to treat this as something that can and will be exploited. Thanks for finding this bug and making everyone aware.

[WietseWind]

Thank you for thinking this through and reporting this here.

While it would be easy to say "then the issuer of the NFT shouldn't set the lsfTrustLine flag", we all know there will be plenty of issuers & projects who won't read the warnings or don't understand the implications.

Especially since we're talking about a public, open blockchain anyone can retrieve all information from, it will be really easy for the malicious to find where to 'attack'. Warnings to issuers/minters/sellers won't be enough, and while due to the account reserve thi may be a semi expensive attack, with shitty projects 'rugpulling' millions, I think we can safely assume the reserved XRP won't matter much if one wants to attack.

In the light of this finding I feel the NFT amendment (XLS20) should be modified to remove the lsfTrustLine flag, and require adding a Trust Line explicitly to be able to receive a certain asset. This is also more in line with what a Trust Line represents: an explicit opt in to be able to receive and hold a specific asset.

[jscottbranson]

Based on these findings, I also support dropping the flag entirely. I see the flag as providing a small convenience at a potentially significant cost to the XRPL and the users thereof.

I feel the XRPL should support intentional use - Anyone can explicitly set trustlines to allow consumers with their NFTs to trade in the trusted IOUs, even if the flag is dropped. However, I question how many businesses are in a legal/regulatory position to accept ANY IOU, and thus the minor convenience offered by the flag is clearly overshadowed by the significant risk the flag poses to any marketplace with it enabled. There are further implications, for example, if a marketplace also has rippling enabled.

[ClubXRPL]

But marketplaces are having NFT collections use their own unique Issuer address in most cases. Not like you all will change your mind, but this seems like a kneejerk reaction that will be yet again another lengthy delay. Also, users do not want to have a trustline for NFT either if that is what this is saying would happen, then we would be in a similar spot to XLS14 no?

[ClubXRPL]

Whatever you guys feel is best, just wish there was a better balance of connecting with the 'mass adoption' side of the NFT implementation. Hoping this change doesn't mean that all users now have to have a trustline for a specific token, vs maybe having the trustline for a full collection. At this point, might as well just do a sidechain for the NFTs and not even have them on the main ledger.

Is there any reason all the validators that were in favor of XLS20 as is decided to vote yay regardless of this issue?

[ftso-eu]

Thank you for having found, pointed out and explained it! We believe that it is not possible in this situation to continue to be in favor of the amendment: xrp ledger safety first, fix later.

[ajkagy]

Well done in finding this and also bringing it to David's attention at the XRPL zone on Friday. Hopefully enough validators will see this and make an informed decision in the next few days.

I also support rabbit's thought about dropping the flag entirely. It should be the issuer's responsibility to add the trusted lines to their account to receive the appropriate transfer fees. It's intentional use.

[xVet]

Good call Mike! Dropping the flag entirely as it goes against the philosophy of a Trustline, is definitely my preferred option as well 💯

[SuperEagleCoin]

I agree with many of the points here. I would add that I can only see issues with allowing people to transact in IOU tokens. Should we not force XRP for NFT transactions? Make the lsfOnlyXRP flag mandatory?

This would always allow royalties to sent to issuers and facilitate successful NFT transfers. It seems like it would be quite onerous to force the issuer to keep curating their trust lines to allow other parties to transact.

[jacobpretorius]

Is there any reason all the validators that were in favor of XLS20 as is decided to vote yay regardless of this issue?

Clearly because the issue was just reported and no one knew about it.

[Silkjaer]

Thanks for reporting this.

I am all for removing the flag. As the current design will just limit the IOUs in which the trades can be made, based on the trustlines that the issuer account currently has set, it will still be tradable. The downside to this is that an issuer (or "original artist" in popular NFT use cases) can control the markets in which his works can be sold in 2nd/3rd/... degree - and change this after the fact of issuing the NFT (unlike tfOnlyXRP).

An alternative solution discussed elsewhere, but summarized here:

• Deliver fees according to current design, in a percentage of XRP used to trade the NFT – or a percentage of the IOU that it was used to trade the NFT, if the issuer has a trustline already for this. This also means the accepted currencies is a "mutable configuration".
• In case the issuer does not have a trustline for an IOU used to trade, pay the fee in XRP using pathfinding similarly to partial payments: Send the same percentage of the IOU, but deliver whatever amount the current market can. If the IOU don't have DEX liquidity, whether the trade had any value is arguable anyway.
• Downside is that two parties can agree to use a worthless IOU, but settle in a different currency OTC – this option would always be possible anyway, as they could just sell the token for a price of 0.

This solution will be just as little friction as auto-creating trustlines, but arguably better as it will also reduce trustline spam on ledger, and bring more utility to XRP. I am not sure how it affects performance.

[jacobpretorius]

I agree with many of the points here. I would add that I can only see issues with allowing people to transact in IOU tokens. Should we not force XRP for NFT transactions? Make the lsfOnlyXRP flag mandatory?

This would always allow royalties to sent to issuers and facilitate successful NFT transfers. It seems like it would be quite onerous to force the issuer to keep curating their trust lines to allow other parties to transact.

I don't think forcing lsfOnlyXRP is a good idea. I can name a few of the current digital art NFT projects which don't have a 1:1 or 0.5:1 ratio in terms of IOU token/coin to redeemable NFT once minting is possible and for these projects a big draw is being able to sell future NFT collections for their coin which would then bring more of their coin back to the project treasury account, along with royalties. It doesn't have to be XRP and that has a lot of appeal.

Also for use cases further away from the NFT = digital artwork space who are looking to use the NFT (which let's all remember is really just a bit of data) for places like schools, universities, or external certifications forcing xrp makes it that much harder for them to get adoption.

[DPMonks]

With the current spam issue on XLS20/IOU’s, Instead of automatically generating a trustline for a IOU to the Issuer on the trade, could it not be simply implemented to offer the issuer the choice to sign to generate the trustline and automatically follow the transaction.

Giving the issuer the option to accept without any major amendments to the network.

point to note, if we removed the IOU capability and didn’t allow issued tokens to function on the XLS20 amendment, we’d not only be removing choice for how users look to transact value but we’d be reducing the network function from a current layer 3 to a layer 1 as issued tokens from other networks do give cross chain capability on the XRPL.

The XRPL would no longer be a layer 3 solution if we just used XRP as it would not be compatible with other blockchain issued tokens (BTC/ETH/SGB/USDC etc)

[Silkjaer]

point to note, if we removed the IOU capability and didn’t allow issued tokens to function on the XLS20 amendment, we’d not only be removing choice for how users look to transact value but we’d be reducing the network function from a current layer 3 to a layer 1 as issued tokens from other networks do give cross chain capability on the XRPL.

Removing the flag does not remove IOU capabilities, but restricts it to the trustlines set by the issuer.

[DPMonks]

point to note, if we removed the IOU capability and didn’t allow issued tokens to function on the XLS20 amendment, we’d not only be removing choice for how users look to transact value but we’d be reducing the network function from a current layer 3 to a layer 1 as issued tokens from other networks do give cross chain capability on the XRPL.

Removing the flag does not remove IOU capabilities, but restricts it to the trustlines set by the issuer.

My apologies, I was reading through quite quick I thought I read something about only using XRP. 👍

[DPMonks]

I had initially thought you could transact all rewards (trade IOU reward to XRP) to the issuer in XRP but demand can be incredibly low on some IOU’s and makes this probably a lot less favourable

[KillaStryder]

Hi Guys, Why not use pathfinding or Rippling to change the transfer fees to XRP once someone pays with a token that the NFToken issuer doesn't own, or completely just do that for every transfer fee received? This might be a way to mitigate the vulnerability rather than setting up trustlines

[KillaStryder]

Good call Mike! Dropping the flag entirely as it goes against the philosophy of a Trustline, is definitely my preferred option as well 💯

Rather than doing that, why not use rippling to change it to XRP once the transaction has successfully gone through? Or if the trustline is already set by the user, sending the transfer fee directly to them with the token being used for the purchase/trade

[Silkjaer]

Hi Guys, Why not use pathfinding or Rippling to change the transfer fees to XRP once someone pays with a token that they don't own or completely just do that for every transfer fee received? This might be a way to mitigate the vulnerability rather than setting up trustlines

https://github.com/XRPLF/rippled/issues/4300#issuecomment-1243354552

[HadaNFT]

I think I've found another possible exploit to avoid paying the transfer fee altogether.

If the secondary sale is done using a XLS14D NFT the transfer fee is not payed as the 14D NFT can not be divided.

[Silkjaer]

If the secondary sale is done using a XLS14D NFT the transfer fee is not payed as the 14D NFT can not be divided.

It can be argued whether that is an exploit or not, since XLS14d tokens are technically misusing the IOU functionality of the XRPL itself. From the technical perspective (think that the XRPL has a consciousness), I'll argue that XLS14d tokens are basically worthless: it's difficult if not impossible to properly trade on DEX or exchange with pathfinding, so the idea of value only exists off-chain.

Personally, XLS14d tokens hopefully dies a quick death as XLS20 is amended.

To avoid fees, the easiest way is probably to "sell for free" and settle the trade over the counter.

[TusharPardhe]

Based on these findings, I also support dropping the flag entirely. I see the flag as providing a small convenience at a potentially significant cost to the XRPL and the users thereof.

I feel the XRPL should support intentional use - Anyone can explicitly set trustlines to allow consumers with their NFTs to trade in the trusted IOUs, even if the flag is dropped. However, I question how many businesses are in a legal/regulatory position to accept ANY IOU, and thus the minor convenience offered by the flag is clearly overshadowed by the significant risk the flag poses to any marketplace with it enabled. There are further implications, for example, if a marketplace also has rippling enabled.

I don't think dropping the flag is a good option rather I suggest to add in a separate field that is mandatory when the flag is on.

A field that specifies which tokens the current issuers wants to accept with their issuer accounts. For eg: If the flag is true this field should be mandatory

acceptableTokens: [ { name: “test”, issuer:”r….” }, { name:”abc”, Issuer:”rkskkk” } ]

[Silkjaer]

I don't think dropping the flag is a good option rather I suggest to add in a separate field that is mandatory when the flag is on.

A field that specifies which tokens the current issuers wants to accept with their issuer accounts. For eg: If the flag is true this field should be mandatory

acceptableTokens: [ { name: “test”, issuer:”r….” }, { name:”abc”, Issuer:”rkskkk” } ]

I think your suggestion would introduce new issues:

1. NFToken size increase, as the flag is specified on the token, the list should also be on the token.
2. If the list should be mutable, the current design with a NFToken pseudo object type won't work
3. If the list is not mutable, gateways are subject to change (a gateway can close, new arrive, issuing addresses change ...)

If the list is not on the NFToken itself, I (almost) don't see any difference from the current design, if the flag is removed or is not set: the "whitelist" is basically the trustlines set on the issuer account.

[Silkjaer]

Just to make it clear, the Trustline flag does not define whether a token can be traded with IOUs or not.

The OnlyXRP flag does that.

The Trustline flag enables auto-creation of trustlines for the issuer account, which is the exploitable bit.

[TusharPardhe]

I don't think dropping the flag is a good option rather I suggest to add in a separate field that is mandatory when the flag is on.

A field that specifies which tokens the current issuers wants to accept with their issuer accounts. For eg: If the flag is true this field should be mandatory

acceptableTokens: [ { name: “test”, issuer:”r….” }, { name:”abc”, Issuer:”rkskkk” } ]

I think your suggestion would introduce new issues:

1. NFToken size increase, as the flag is specified on the token, the list should also be on the token.
2. If the list should be mutable, the current design with a NFToken pseudo object type won't work
3. If the list is not mutable, gateways are subject to change (a gateway can close, new arrive, issuing addresses change ...)

If the list is not on the NFToken itself, I (almost) don't see any difference from the current design, if the flag is removed or is not set: the "whitelist" is basically the trustlines set on the issuer account.

My suggestion was your first point to introduce a list in each individual mint.

Your point makes sense but I didn't understand the "whitelist" thing maybe I'm missing something.

[Silkjaer]

Now I see it your point makes sense but didn't understood the "whitelist" thing maybe I'm missing something

If the issuer has set 10 different trustlines on her account, she can receive fees in those 10 different IOUs (+ XRP). If she wants support for more, she adds more trustlines to her account. So the current design (almost) does what you suggest. Without increasing object sizes and keeping mutability.

[TusharPardhe]

Now I see it your point makes sense but didn't understood the "whitelist" thing maybe I'm missing something

If the issuer has set 10 different trustlines on her account, she can receive fees in those 10 different IOUs (+ XRP). If she wants support for more, she adds more trustlines to her account. So the current design (almost) does what you suggest. Without increasing object sizes and keeping mutability.

That solves it! 🙂

[Silkjaer]

That solves it! 🙂

Great, that's the existing design without the flag 😉

[ClubXRPL]

Hi Guys, Why not use pathfinding or Rippling to change the transfer fees to XRP once someone pays with a token that the NFToken issuer doesn't own, or completely just do that for every transfer fee received? This might be a way to mitigate the vulnerability rather than setting up trustlines

In this case though isn't pathfinding the same as the buyer just selling their 'iou' for xrp anyways?

[myalcin81]

The existing design is already working well without the flag field. Conversations in the thread turn out the feature implementation instead of fixing the bug. Keep it simple and fix the bug, if you have any other recommendations over it, create a pull request or a discussion topic.

[KillaStryder]

I'm now in agreement to remove the flag as a whole. We can always work on new flags as part of a sidechain feature, rather than messing up a good thing here.

[Crypto-Python]

I'm glad this is discussed. On our project, I didn't know what to do with this flag. As we want to have the royalties as close to FIAT as we can, for tax and other reasons, in any case, we were planning on using the DEX to convert everything. The plan was to set this flag, and have some reserves to cover it. When NFT is sold, royalties come in. If enough tokens are accumulated, a script gets the list for liquidity of the token we received. Create an offer to convert all tokens. Delete trustline. Repeat with the next royalty. Effectively keeping the trustline number low and getting back the reserves. Though if there is an attack this would probably escalate quickly, the two scripts fighting it out, and the hackers can attack with multiple accounts.

[intelliot]

@x-Tokenize can you confirm that this issue has been fixed by the fixRemoveNFTokenAutoTrustLine amendment?

(fixRemoveNFTokenAutoTrustLine was enabled Oct 27, 2022, 04:09:30 PM UTC)

[kennyzlei]

@x-Tokenize I am closing this issue and marking as fixed based on the fixRemoveNFTokenAutoTrustLine amendment introduced in https://github.com/XRPLF/rippled/pull/4301. Feel free to comment and reopen if you believe this fix does not address the issue you opened here