synonymdev / bitkit

Self-custodial Bitcoin and Lightning Wallet for Android and iOS.
https://bitkit.to
MIT License
115 stars 23 forks source link

[Bug]: Advanced Manual Setup for LN channel can result in crash #2253

Open catch-21 opened 5 days ago

catch-21 commented 5 days ago

Describe the bug

Bitkit crashes if the Manual Setup flow form is not filled in and the Continue button is pressed. (This may have persistently broken ldk, need to test more)

Reproduce

  1. Go to Advanced settings -> Lightning Connections -> Add Connection
  2. At Manual Setup page, tap 'Continue'
  3. App crashes
  4. On restarting Bitkit the LDK node never starts correctly (wallet restore required)

Screenshots / Recording

https://github.com/user-attachments/assets/16a55c09-859a-456a-a216-5bfa49a2a463

https://github.com/user-attachments/assets/df133def-72c0-4f85-bdc1-5d945cce034b

Operating system

iOS / Android

Bitkit version

1.0.5 (136)

Log output

crash-report.txt

pwltr commented 4 days ago

Can confirm, the app is crashing when trying to add a peer with an invalid pubkey. I can add validation to check for correct length but that's not enough to prevent this. @Jasonvdb FYI

https://github.com/user-attachments/assets/16a55c09-859a-456a-a216-5bfa49a2a463

crash-report.txt

pwltr commented 4 days ago

Crashing on Android as well:

https://github.com/user-attachments/assets/df133def-72c0-4f85-bdc1-5d945cce034b

Jasonvdb commented 4 days ago

Yeah length check doesn't seem like enough here. Needs proper validation.

Jasonvdb commented 4 days ago

This should do the trick: https://github.com/synonymdev/react-native-ldk/pull/274

I'm testing with 0000000000000000000000000000000000000000000000000000000000000000

catch-21 commented 1 day ago

@pwltr It's interesting you get a crash on android because I see a toast (real device)

A fix must be tested on both platforms

Jasonvdb commented 3 hours ago

Still strangely stopped being able to replicate the crash since Friday but I believe this does the trick: https://github.com/synonymdev/bitkit/pull/2275

Using the actual curve to validate the public key.