Skip to content

Add test for push_asset_amount underflow DoS on funding acceptor - #138

Merged
zoedberg merged 2 commits into
RGB-Tools:masterfrom
0xaudron:fix-underflow-dos
Jul 30, 2026
Merged

Add test for push_asset_amount underflow DoS on funding acceptor#138
zoedberg merged 2 commits into
RGB-Tools:masterfrom
0xaudron:fix-underflow-dos

Conversation

@0xaudron

Copy link
Copy Markdown
Contributor

The acceptor computed remote_rgb_amount = channel_rgb_amount - push_amount without checking push_amount <= channel_rgb_amount, so a counterparty sending a larger value underflowed it (release builds have overflow checks off), aborting RGB coloring and panicking the acceptor's event handler.

The REST layer clamps push_asset_amount, so reaching this needs a modified wire client; FORCE_WIRE_PUSH_ASSET_AMOUNT_ON_NODE models that by making the initiator emit an out-of-range value on the wire. The test asserts the acceptor rejects the funding and stays responsive.

Bumps rust-lightning to the commit rejecting out-of-range push_asset_amount in handle_funding.

@zoedberg zoedberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I requested some changes, also this branch has conflicts that must be resolved. Finally please do not commit the updated submodule pointer, I first would like to see the CI fail, then I'll push your fix on the upstream remote so you will be able to change the pointer without any change to the .gitmodules file

Comment thread src/ldk.rs Outdated
Comment thread src/routes.rs Outdated
Comment thread src/test/push_asset_amount_underflow.rs Outdated
Comment thread src/test/push_asset_amount_underflow.rs Outdated
Comment thread src/routes.rs Outdated
@0xaudron

Copy link
Copy Markdown
Contributor Author

Addressed all review points: FORCE_PUSH_ASSET_AMOUNT_ON_NODE is now a simple on/off flag like IGNORE_INBOUND_CHANNELS_ON_NODE and uses NodeOverrideGuard (custom Drop removed), test and file renamed to push_asset_amount_above_chan_amt, the redundant connect_peer is gone, and the explanation lives only on the constant. Rebased on master and dropped the submodule pointer.

CI should now fail on the new test, ready for you to push the LDK fix upstream.

@zoedberg

Copy link
Copy Markdown
Member

@0xaudron the CI cannot run because the branch has still conflicts that must be resolved. You should rebase this on top of the latest master tip. Please before the rebase also squash the commits into a single one

@0xaudron
0xaudron force-pushed the fix-underflow-dos branch from d4a1065 to 6be3835 Compare July 29, 2026 16:17
@0xaudron
0xaudron requested a review from zoedberg July 29, 2026 17:49
@0xaudron

Copy link
Copy Markdown
Contributor Author

@zoedberg can you check it now ?

@zoedberg

Copy link
Copy Markdown
Member

LGTM, I'm waiting for your rebase in RGB-Tools/rust-lightning#33 , after that I'll push the branch on the public repo so you can add a commit here that updates the submodule pointer

@0xaudron

Copy link
Copy Markdown
Contributor Author

done @zoedberg

@zoedberg

Copy link
Copy Markdown
Member

@0xaudron I pushed the branch, you can now add a commit here that updates the submodule pointer

@zoedberg

Copy link
Copy Markdown
Member

@0xaudron I will add the commit myself, so we can then proceed to merging this

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.77%. Comparing base (463b244) to head (ce4a123).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #138      +/-   ##
==========================================
- Coverage   87.78%   87.77%   -0.01%     
==========================================
  Files          13       13              
  Lines        7613     7617       +4     
==========================================
+ Hits         6683     6686       +3     
- Misses        930      931       +1     
Flag Coverage Δ
rust 87.77% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zoedberg
zoedberg merged commit ce4a123 into RGB-Tools:master Jul 30, 2026
12 checks passed
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