Fix: reject fake scids with invalid vout
What the developer wrote
Authored by Valentine Wallace
Fix: reject fake scids with invalid vout
Previously, we would spuriously allow fake scids that had a vout with the high
byte set to pass our is_valid_{phantom,intercept,etc}_scid checks, even though
our fake vouts only ever set the lowest 3 bits of the 2-byte vout.
This can't really be exploited since HTLCs that pass this check would still
fail later on in the pipeline, and attackers that want to craft fake scids to
pass our checks can still do so after this fix, either via brute force or by
reusing a valid fake scid from a previously issued invoice. But at least this
makes it harder for them to do so, and makes the check more correct than it was
before. Plus invalid fake crafted scids like this could theoretically cause us
to generate a spurious HTLCIntercepted event, which wouldn't be ideal.
Reported by Project Loupe.
What changed, and why it matters
This commit fixes a validation bug in how Lightning Dev Kit checks 'fake' short channel IDs (SCIDs) used for routing tricks like phantom and intercept payments. The old check compared only the lowest byte of the vout field, so an attacker could set the high byte of the vout and still pass validation. The fix now compares the full 16-bit vout value. The project says this is not directly exploitable for stealing funds, but it could cause a spurious internal event and makes the validation more correct.
Apply the patch. Review any other fake-SCID namespace validators for similar u16/u8 truncation issues. Consider whether downstream consumers that rely on HTLCIntercepted events need hardening against spurious events.
Security signals we found
Input validation bypass in fake SCID checks
Type-cast truncation bug (u16 to u8) leading to incorrect equality check
Potential for spurious HTLCIntercepted event generation
Reported by external security research group (Project Loupe)
Evidence from the diff
The functions is_valid_phantom_scid and is_valid_intercept_scid in lightning/src/util/scid_utils.rs previously compared an encrypted vout (u8) against scid_utils::vout_from_scid(scid) cast to u8. Because vout_from_scid returns a u16 and the cast discarded the high byte, any SCID whose low byte matched the expected encrypted vout would pass, even if the high byte was non-zero. The patch changes the comparison to valid_vout as u16 == scid_utils::vout_from_scid(scid), so the full 16-bit vout must match. Tests are added for both phantom and intercept namespaces to reject SCIDs with the high byte set.
Changed components
lightning/src/util/scid_utils.rsis_valid_phantom_scidis_valid_intercept_scidfake_scid namespace validationInspect captured patch +20 / −2
diff --git a/lightning/src/util/scid_utils.rs b/lightning/src/util/scid_utils.rs
index 342c062..c5a9182 100644
--- a/lightning/src/util/scid_utils.rs
+++ b/lightning/src/util/scid_utils.rs
@@ -180,7 +180,7 @@ pub(crate) mod fake_scid {
let namespace = Namespace::Phantom;
let valid_vout = namespace.get_encrypted_vout(block_height, tx_index, fake_scid_rand_bytes);
block_height >= segwit_activation_height(chain_hash)
- && valid_vout == scid_utils::vout_from_scid(scid) as u8
+ && valid_vout as u16 == scid_utils::vout_from_scid(scid)
}
/// Returns whether the given fake scid falls into the intercept namespace.
@@ -192,7 +192,7 @@ pub(crate) mod fake_scid {
let namespace = Namespace::Intercept;
let valid_vout = namespace.get_encrypted_vout(block_height, tx_index, fake_scid_rand_bytes);
block_height >= segwit_activation_height(chain_hash)
- && valid_vout == scid_utils::vout_from_scid(scid) as u8
+ && valid_vout as u16 == scid_utils::vout_from_scid(scid)
}
#[cfg(test)]
@@ -248,6 +248,15 @@ pub(crate) mod fake_scid {
assert!(is_valid_phantom(&fake_scid_rand_bytes, valid_fake_scid, &testnet_genesis));
let invalid_fake_scid = scid_utils::scid_from_parts(1, 0, 12).unwrap();
assert!(!is_valid_phantom(&fake_scid_rand_bytes, invalid_fake_scid, &testnet_genesis));
+ // A scid whose low byte matches the namespace value but whose high byte is set must be
+ // rejected (this was previously broken).
+ let high_byte_fake_scid =
+ scid_utils::scid_from_parts(1, 0, valid_encrypted_vout as u64 | 0x0100).unwrap();
+ assert!(!is_valid_phantom(
+ &fake_scid_rand_bytes,
+ high_byte_fake_scid,
+ &testnet_genesis
+ ));
}
#[test]
@@ -265,6 +274,15 @@ pub(crate) mod fake_scid {
invalid_fake_scid,
&testnet_genesis
));
+ // A scid whose low byte matches the namespace value but whose high byte is set must be
+ // rejected (this was previously broken).
+ let high_byte_fake_scid =
+ scid_utils::scid_from_parts(1, 0, valid_encrypted_vout as u64 | 0x0100).unwrap();
+ assert!(!is_valid_intercept(
+ &fake_scid_rand_bytes,
+ high_byte_fake_scid,
+ &testnet_genesis
+ ));
}
#[test]
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.