Rippled: PartialPayment feature resulting in integration bugs

[nixer89]

Hi all,

the documentation for the XRPL states:
"Partial payments can be used to exploit naive integrations with the XRP Ledger to steal money from exchanges and gateways." https://xrpl.org/partial-payments.html

This seems to be a big problem lately. Many exchanges did not integrate the XRPL properly and are/were vulnerable to PartialPayments. This is such a big issue, that pointing to the XRPL-documentation seems not to be enough.

It would be useful to add a new AccountRoot flag "lsfAllowPartialPayment" which defaults to "false".
This way, sending a partial payment transaction is failing with "tefPARTIAL_PAYMENT_DISALLOWED" on accounts which not actively allowed PartialPayments.
Allowing/Disallowing PartialPayments can be done by an AccountSet-Transaction with a new Flag: "asfAllowPartialPayment".

The benefits are:

• much easier integration of XRP into other systems
• partial payments cannot be exploited anymore (if an account has disabled them)

I think disallowing them by default would benefit a big user base of XRP and negatively affect just a few.

[yxxyun]

there was a plan to do so.
https://github.com/nbougalis/rippled/commit/52d04bfcb0f63ffad58fdd495c036497c78f695c

https://github.com/ximinez/rippled/commits/nb-ppt2

[nixer89]

Is there any chance this can make it as an amandment?
I saw it was already developed back in August 2018 but I actually never heard of it nor it is a known amandment yet.

[yxxyun]

some discuss here, https://www.xrpchat.com/topic/26897-reject-incoming-partial-payments-by-default/

but don't know when will be merged.

[MarkusTeufelberger]

Partial payments are a very useful feature and defaulting it to "off" would mean a very drastic change for every single (existing!) account on the ledger that is unexpected and unilateral - users have no way to veto this.

I'd like to see some concrete numbers for your claims that: "This seems to be a big problem lately. Many exchanges did not integrate the XRPL properly and are/were vulnerable to PartialPayments. This is such a big issue, that pointing to the XRPL-documentation seems not to be enough."

1) Why is this a problem "lately", if the first public case was about 5 years ago? https://xrpl.org/blog/2014/gateway-advisory-on-partial-payment-flag.html is old enough that Ripple even removed their whole wiki since then. (It probably should link to https://ripple.com/files/GB-2014-06.pdf and make these a bit more prominent @mDuo13 ?)
2) How many off-chain exchanges were affected so far?
3) How many are still vulnerable?

Cutting out useful features because people copy-paste code or don't read documentation when handling customer funds (and don't publish their code or pay someone competent to audit it) is a weak reason in my book - after all, PartialPayments are especially useful for someone to SEND, not exactly something you need to think about to RECEIVE. There's a big chance that people will just leave this at the default, essentially killing the feature (just like the NoFreeze flag, which used to be default and then got pushed onto the network unilaterally).

[nixer89]

Not relating to exchanges only.
Recent vulnerabilities (exploited or not) were BitPro (exchange), R3 CordaSettler, Beaxy (exchange). And I know the XRPCommunity "tested" other exchanges and reported if they were vulnerable.

I agree with you that a default to "disallow" partial payments could kill the feature. So maybe we should think of another way to mitigate the problem. In @yxxyun's link there was a suggestion of a separate "PartialPayment" transaction. Maybe this could be a solution?! (I just want to force some discussion to this topic here).

What I disagree with you is blaming people "not reading" the documentation. You might be right that they should have done their audit and find proper "XRPL"-Integrators and they may should have read the documentation. But this happened many times now already.
What if adaoption really takes place?
At the moment we are quite early adoption wise. If those few adopers already make many mistakes in this regards, what if more and more people will adopt the XRPL?

There seems to be a problem with partial payments and that they can be used to exploit bugs in 3rd party software. It happened 5 years ago and it still happens nowadays.

I think the XRPCommunity should think of a way to make the integration of the XRP-Ledger more easy and more fail safe. Where people work, mistakes are going to happen. Blaming them for not reading the docs does not help them nor the customers -> it will be a bad experience for both and that is something we need to mitigate.

[MarkusTeufelberger]

So far there has been about 1 case per year of someone implementing things incorrectly. I wouldn't call this "many" (or "quite early" for that matter). There are a LOT more exchanges and service providers that implemented this correctly over the years, some errors will always happen just like you can trivially write a buffer overflow bug in C. If you want a walled garden or sandbox with rounded corners and limited functionality, then maybe XRP is not the right choice?

Also there sorta is already a separate "PartialPayment" transaction: It is a Payment transaction that has the tfPartialPayment flag enabled. Since the handling on the receiving end is just the same as for non-partial payments (look at the delivered amount and credit the user) I don't see many benefits in renaming this type of transaction to make it into its own thing...

[mDuo13]

Any change to the way partial payments work today is likely to require changes for the (many!) institutions who have correctly implemented them today. And, although Ripple does not push the "gateway" business idea much these days, it's still something that can be done with the XRP Ledger, and partial payments are more relevant for that use case for returning issued currency payments. Breaking partial payments (e.g. by the proposed change of making it so new accounts can't receive them unless they opt in) would especially hurt the businesses that use the XRP Ledger's decentralized exchange today.

I'm always adding more warnings to the documentation about this issue, but that only helps people who read the documentation. Those who read the API messages and assume how they work, I can't help.

[intelliot]

To reiterate: The solution is to always use the deliveredAmount (or delivered_amount) field to determine the amount that the transaction delivered. When using the recommended higher-level library ripple-lib, it's infeasible for this bug to occur because ripple-lib automatically hides the amount field, leaving the deliveredAmount as the only available field to check.

(As of ripple-lib 1.0.0, 2018-08-30)

[nbougalis]

@MarkusTeufelberger, you wrote:

_Partial payments are a very useful feature_ […]

I agree. I do not intend to offer code that removes the partial payment feature altogether and would argue against incorporating such a change if it was proposed by someone else.

[…] _and defaulting it to "off" would mean a very drastic change for every single (existing!) account on the ledger that is unexpected and unilateral - users have no way to veto this._

Not correct. My exploratory change, which was linked above, does not have any impact on existing accouts and would only affect accounts created after the amendment was enabled; accounts created before would have to explicitly opt-in by setting the flag manually.