RGB-Tools / RGB-Tools/rgb-lightning-node

Channel artifacts and ID mappings are keyed by bare IDs with no owner binding

Open
#158 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
50
Forks
48
Avg merge
8h 52m
Merged PRs (30d)
2

Description

Summary

RGB channel metadata, consignments, funding PSBTs and the temporary→final channel-ID map are all stored under a bare channel ID or funding txid. Nothing records which peer or which open attempt owns an entry, and the checks that do exist only look at the node's own former-temporary-ID map or at a single peer's channel set. As a result, an ID chosen by an API caller (or derived from a peer-supplied message) can collide with an entry that belongs to a different live channel, and the next step — funding, cleanup or close — overwrites, deletes or misreads the other channel's data without any error.

Details

1. A caller-chosen temporary channel ID is only checked against the former-temp-ID map

POST /openchannel accepts an optional temporary_channel_id. The only collision checks are channel_ids().contains_key(id) (the keys of the former-temporary → final map) and the virtual-channel draft store. The ID is not compared against:

  • final channel IDs of live channels (the values of that map, and channels whose mapping has already been removed),
  • temporary IDs of in-flight opens with other peers — LDK's create_channel only checks peer_state.channel_by_id for the selected peer.

For a colored open, open_channel then writes rgb_channel_info (pending and non-pending) under that ID before calling create_channel, and removes both entries if create_channel fails. So an ID equal to another channel's ID overwrites that channel's RGB balance record; the failure path deletes it. Both the initial commitment coloring and every later HTLC coloring read that record, so the live channel now carries the wrong asset/amount.

2. The reverse lookup in the ID map removes the first match

delete_channel_id(final_id) walks the map and removes the first temporary key whose value equals final_id. add_channel_id (called from ChannelPending) inserts blindly and overwrites any existing mapping for that key. If two temporary IDs end up mapping to the same final ID (retried or replayed ChannelPending, stale entries after a crash), the wrong entry can be removed and the stale one survives. save_channel_ids_map unwrap()s the KV write while holding the map mutex, so a single write error poisons the mutex for every later channel operation.

3. Acceptor-side artifacts are keyed by txid only, and the funding vout is hard-coded

In handle_funding the consignment fetched from the peer's proxy endpoint is stored under funding_txid and under the temporary channel ID, and rgb_channel_info is written under the temporary ID — none of them carries the peer or the funding output index. _accept_transfer calls wallet.accept_transfer(funding_txid, funding_vout = 1, …) regardless of msg.funding_output_index, so the RGB allocation is bound to output 1 whatever the actual funding output is. ChannelPending later decides initiator / acceptor / vanilla purely by whether a PSBT or a consignment exists under the txid.

4. The initiator derives the final channel ID from the first matching output script

In FundingGenerationReady the final channel ID used as the key for the PENDING_FUNDING record is derived from output.iter().position(|o| o.script_pubkey == script_buf) cast to u16, independently of the index LDK actually selects for the channel.

How it can be triggered

  1. Open a colored channel A with peer P1 and let it fund.
  2. Call POST /openchannel for peer P2 with temporary_channel_id = A's channel ID (temporary or final), any asset.
  3. RLN writes rgb_channel_info for A's ID with B's asset/amounts before create_channel; if P2 rejects the open (or create_channel errors), the metadata for A is deleted.

The failing-open-cleanup and close paths (delete_channel_id, handle_open_chan_fail) then act on whatever entry matches first, not on the entry owned by the closing channel.

Impact

  • Overwrite or deletion of the RGB balance record of a live, funded channel; later commitment coloring uses the wrong asset/amount.
  • Close / failed-open cleanup removes the mapping of — or fails the batch transfer of — a different channel.
  • Acceptor RGB state attached to the wrong funding output; the transfer lookup and sweeper after close then fail or misclassify the output as plain BTC.
  • A KV write failure during cleanup leaves a poisoned mutex; every subsequent channel operation panics until restart.

The collision itself is reproducible through the REST API. The end-to-end asset-loss consequence has not been executed yet and should be validated with a funded test.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Trace POST /openchannel and the channel lifecycle entry points named in the issue: open_channel, handle_funding, FundingGenerationReady, ChannelPending, delete_channel_id, and handle_open_chan_fail. Identify every artifact and mapping key that lacks peer, attempt, or output ownership, then validate collision, cleanup, and funding-output behavior with the funded test requested in the issue. Done means conflicting channels cannot overwrite or misread one another and KV write failures do not poison later operations.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend-api-design, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.