lnd icon indicating copy to clipboard operation
lnd copied to clipboard

Bad route hint sends sender node into infinite payment loop

Open joostjager opened this issue 3 years ago • 10 comments
trafficstars

Your environment

  • lnd master 2682ca3e22ded37aa75ba40b5dd826bcd5cdd20a
  • linux
  • bitcoind

Steps to reproduce

  • Nodes A and B with a channel between them
  • Patch node B to generate invoices that include a hop hint from A via a non-existent channel id
  • Create invoice on B
  • Pay invoice from A

Expected behaviour

Payment fails immediately.

Actual behaviour

Node A goes into an infinite payment loop where each iteration the outcome is the same UNKNOWN_NEXT_PEER @ 0.

joostjager avatar Jan 17 '22 13:01 joostjager

Is this a new behavior?

alexbosworth avatar Jan 18 '22 18:01 alexbosworth

I don't know. Also, maybe B can do this to any node, not just the ones that they have a channel with.

joostjager avatar Jan 18 '22 18:01 joostjager

The infinite loop is still bounded by the pathfinding timeout though right?

alexbosworth avatar Jan 18 '22 18:01 alexbosworth

Didn't actually wait for that, but very likely that it is still bounded.

joostjager avatar Jan 18 '22 18:01 joostjager

@joostjager Me and a friend (@andreihod) were able to reproduce this bug reliably and we think we know how to fix it. Is it okay for us to work on it? I'm asking in case someone is already looking into this.

lsunsi avatar Jul 18 '22 23:07 lsunsi

I did find something, not sure if it's related.

Is it okay for us to work on it? I'm asking in case someone is already looking into this.

@lsunsi if it's a small fix then I'd say go for it! What's the cause you guys found?

yyforyongyu avatar Jul 19 '22 00:07 yyforyongyu

@yyforyongyu Nice! We think it's unrelated to your findings: it seems that the processPaymentOutcomeIntermediate was treating the resulting error from this bug while the processPaymentOutcomeSelf was not, leading to the infinite loop because the failure is not marked as final by the handler.

We could verify this because we were able to reproduce the bug easily with a payment where the broken hint is the first hop, but could not reproduce with a payment where the broken hint is the second hop. This different behaviour is what lead us to look into the aforementioned functions.

Does that make sense?

lsunsi avatar Jul 20 '22 22:07 lsunsi

@lsunsi Thanks for the detailed explanation! Yeah in this case I'd say please go ahead and make a PR!

yyforyongyu avatar Jul 22 '22 12:07 yyforyongyu

@saubyk This is a P2 bug with a 3-line (ex tests) fix in https://github.com/lightningnetwork/lnd/pull/6766. Move to 0.16 milestone?

joostjager avatar Oct 21 '22 12:10 joostjager

Thanks @joostjager for bubbling this up. Tagged for 0.16.0

saubyk avatar Oct 21 '22 14:10 saubyk