Skip to content

Reject out-of-range push_asset_amount in handle_funding - #33

Open
0xaudron wants to merge 1 commit into
RGB-Tools:rgbfrom
0xaudron:fix/validate-push-asset-amount
Open

Reject out-of-range push_asset_amount in handle_funding#33
0xaudron wants to merge 1 commit into
RGB-Tools:rgbfrom
0xaudron:fix/validate-push-asset-amount

Conversation

@0xaudron

Copy link
Copy Markdown

handle_funding computes the acceptor side RGB split as remote_rgb_amount = channel_rgb_amount - push_amount, where push_amount comes from the counterparty supplied push_asset_amount in open_channel. The value is not validated against channel_rgb_amount.

When a peer sends push_asset_amount greater than the funded asset amount the subtraction underflows. Release builds have overflow checks off, so the value wraps. The wrapped remote_rgb_amount is then passed to color_psbt, which returns InvalidColoringInfo, and the result is unwrapped. The panic poisons the channel manager mutex and the node stops serving channel operations until it is restarted.

The initiator side already rejects this in rgb-lightning-node before sending but the acceptor cannot rely on the counterparty running that check.

Fix: Reject push_amount > channel_rgb_amount in handle_funding before the subtraction, using the existing ChannelError::close path.

@zoedberg

Copy link
Copy Markdown
Member

Thank you! Could you add a test in RLN to prove the attack?

@0xaudron

0xaudron commented Jul 28, 2026

Copy link
Copy Markdown
Author

Sure, btw would you prefer

  • integration test for the PoC which willl require me to push changes to RLN repo separately
  • unit test which will settle new push to rust-ligntning repo

@zoedberg

Copy link
Copy Markdown
Member

Since we have a long-term plan to drop the LDK fork (by introducing the required flexibility upstream), we do not run or write unit tests there. So please write an integration test in RLN.

@0xaudron

Copy link
Copy Markdown
Author

Opened a pull request in RLN: RGB-Tools/rgb-lightning-node#138

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants