Skip to content

Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
valentinewallace merged 3 commits into from
Aug 15, 2022
Merged

Fix payment onion payload decode #1659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dcef41d
Minor msgs::OnionHopData cleanups
valentinewallace Aug 6, 2022
a23681f
Fix bug in onion payment payload decode
valentinewallace Aug 6, 2022
dfbebbf
Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize
valentinewallace Aug 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 59 additions & 28 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
}
}

impl Readable for FinalOnionHopData {
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
let secret: [u8; 32] = Readable::read(r)?;
let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
},
OnionHopDataFormat::NonFinalNode { short_channel_id } => {
encode_varint_length_prefixed_tlv!(w, {
(2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
(2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
(4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
(6, short_channel_id, required)
});
},
OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
encode_varint_length_prefixed_tlv!(w, {
(2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
(2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
(4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
(8, payment_data, option),
(5482373484, keysend_preimage, option)
});
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
<Self as Readable>::read(r)
}
}

impl Readable for OnionHopData {
fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
use bitcoin::consensus::encode::{Decodable, Error, VarInt};
let v: VarInt = Decodable::consensus_decode(&mut r)
.map_err(|e| match e {
Error::Io(ioe) => DecodeError::from(ioe),
_ => DecodeError::InvalidValue
})?;
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
let b: BigSize = Readable::read(r)?;
const LEGACY_ONION_HOP_FLAG: u64 = 0;
let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
let mut rd = FixedLengthReader::new(r, v.0);
let mut amt = HighZeroBytesDroppedVarInt(0u64);
let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
let mut rd = FixedLengthReader::new(r, b.0);
let mut amt = HighZeroBytesDroppedBigSize(0u64);
let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
let mut short_id: Option<u64> = None;
let mut payment_data: Option<FinalOnionHopData> = None;
let mut keysend_preimage: Option<PaymentPreimage> = None;
// The TLV type is chosen to be compatible with lnd and c-lightning.
decode_tlv_stream!(&mut rd, {
(2, amt, required),
(4, cltv_value, required),
(6, short_id, option),
(8, payment_data, option),
// See https://github.com/lightning/blips/blob/master/blip-0003.md
(5482373484, keysend_preimage, option)
});
rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
<Self as Readable>::read(r)
}
}

impl Writeable for Ping {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
use bitcoin::secp256k1::{PublicKey,SecretKey};
use bitcoin::secp256k1::{Secp256k1, Message};

use io::Cursor;
use io::{self, Cursor};
use prelude::*;
use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
}

#[test]
fn decode_onion_hop_data_len_as_bigsize() {
// Tests that we can decode an onion payload that is >253 bytes.
// Previously, receiving a payload of this size could've caused us to fail to decode a valid
// payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
// (little-endian).

// Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
// payload length to be encoded over multiple bytes rather than a single u8.
let big_payload = encode_big_payload().unwrap();
let mut rd = Cursor::new(&big_payload[..]);
<msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
}
// see above test, needs to be a separate method for use of the serialization macros.
fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
use util::ser::HighZeroBytesDroppedBigSize;
let payload = msgs::OnionHopData {
format: OnionHopDataFormat::NonFinalNode {
short_channel_id: 0xdeadbeef1bad1dea,
},
amt_to_forward: 1000,
outgoing_cltv_value: 0xffffffff,
};
let mut encoded_payload = Vec::new();
let test_bytes = vec![42u8; 1000];
if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
(1, test_bytes, vec_type),
(2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
(4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
(6, short_channel_id, required)
});
}
Ok(encoded_payload)
}
}
10 changes: 5 additions & 5 deletions lightning/src/util/ser.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
writer.write_all(&self.to_be_bytes())
}
}
impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
#[inline]
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
// Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
Ok(<$val_type>::from_be_bytes(buf))
}
}
impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
#[inline]
fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
// We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
// the high bytes being dropped. To do so, we start reading into the middle of buf
// and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
let first_byte = $len - ($len - total_read_len);
let mut bytes = [0; $len];
bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
} else {
// If the encoding had extra zero bytes, return a failure even though we know
// what they meant (as the TLV test vectors require this)
Expand Down
10 changes: 5 additions & 5 deletions lightning/src/util/ser_macros.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
use io::{self, Cursor};
use prelude::*;
use ln::msgs::DecodeError;
use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
use bitcoin::secp256k1::PublicKey;

// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
}

// BOLT TLV test cases
fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
let mut s = Cursor::new(s);
let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
let mut tlv2: Option<u64> = None;
let mut tlv3: Option<(PublicKey, u64, u64)> = None;
let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

stream.0.clear();
encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

stream.0.clear();
encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

Ok(())
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Fix payment onion payload decode by valentinewallace · Pull Request #1659 · lightningdevkit/rust-lightning · GitHub
Skip to content
Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

valentinewallace
merged 3 commits into

from

Aug 15, 2022
 Merged


Fix payment onion payload decode #1659
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
dcef41d

Minor msgs::OnionHopData cleanups


valentinewallace Aug 6, 2022
a23681f

Fix bug in onion payment payload decode


valentinewallace Aug 6, 2022
dfbebbf

Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize


valentinewallace Aug 7, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


87 changes: 59 additions & 28 deletions
lightning/src/ln/msgs.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
	}
}

impl Readable for FinalOnionHopData {
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let secret: [u8; 32] = Readable::read(r)?;
 let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
 let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
 Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
	}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
 },
 OnionHopDataFormat::NonFinalNode { short_channel_id } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 },
 OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (8, payment_data, option),
 (5482373484, keysend_preimage, option)
 });
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Readable for OnionHopData {
	fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
 use bitcoin::consensus::encode::{Decodable, Error, VarInt};
 let v: VarInt = Decodable::consensus_decode(&mut r)
 .map_err(|e| match e {
 Error::Io(ioe) => DecodeError::from(ioe),
 _ => DecodeError::InvalidValue
 })?;
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let b: BigSize = Readable::read(r)?;
 const LEGACY_ONION_HOP_FLAG: u64 = 0;
 let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, v.0);
 let mut amt = HighZeroBytesDroppedVarInt(0u64);
 let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
 let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, b.0);
 let mut amt = HighZeroBytesDroppedBigSize(0u64);
 let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 let mut short_id: Option<u64> = None;
 let mut payment_data: Option<FinalOnionHopData> = None;
 let mut keysend_preimage: Option<PaymentPreimage> = None;
 // The TLV type is chosen to be compatible with lnd and c-lightning.
 decode_tlv_stream!(&mut rd, {
 (2, amt, required),
 (4, cltv_value, required),
 (6, short_id, option),
 (8, payment_data, option),
 // See https://github.com/lightning/blips/blob/master/blip-0003.md
 (5482373484, keysend_preimage, option)
 });
 rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Writeable for Ping {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
	use bitcoin::secp256k1::{PublicKey,SecretKey};
	use bitcoin::secp256k1::{Secp256k1, Message};

	use io::Cursor;
	use io::{self, Cursor};
	use prelude::*;
	use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
 assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
	}

	#[test]
	fn decode_onion_hop_data_len_as_bigsize() {
 // Tests that we can decode an onion payload that is >253 bytes.
 // Previously, receiving a payload of this size could've caused us to fail to decode a valid
 // payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
 // (little-endian).

 // Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
 // payload length to be encoded over multiple bytes rather than a single u8.
 let big_payload = encode_big_payload().unwrap();
 let mut rd = Cursor::new(&big_payload[..]);
 <msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
	}
	// see above test, needs to be a separate method for use of the serialization macros.
	fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
 use util::ser::HighZeroBytesDroppedBigSize;
 let payload = msgs::OnionHopData {
 format: OnionHopDataFormat::NonFinalNode {
 short_channel_id: 0xdeadbeef1bad1dea,
 },
 amt_to_forward: 1000,
 outgoing_cltv_value: 0xffffffff,
 };
 let mut encoded_payload = Vec::new();
 let test_bytes = vec![42u8; 1000];
 if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
 encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
 (1, test_bytes, vec_type),
 (2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 }
 Ok(encoded_payload)
	}
}

10 changes: 5 additions & 5 deletions
lightning/src/util/ser.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
	($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
 writer.write_all(&self.to_be_bytes())
 }
 }
 impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 // Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
 Ok(<$val_type>::from_be_bytes(buf))
 }
 }
 impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
 // We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
 // the high bytes being dropped. To do so, we start reading into the middle of buf
 // and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
 let first_byte = $len - ($len - total_read_len);
 let mut bytes = [0; $len];
 bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
 Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
 Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
 } else {
 // If the encoding had extra zero bytes, return a failure even though we know
 // what they meant (as the TLV test vectors require this)
Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/util/ser_macros.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
	use io::{self, Cursor};
	use prelude::*;
	use ln::msgs::DecodeError;
	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
	use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
	use bitcoin::secp256k1::PublicKey;

	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
	}

	// BOLT TLV test cases
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 let mut s = Cursor::new(s);
 let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
 let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
 let mut tlv2: Option<u64> = None;
 let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
 assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 Ok(())
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix payment onion payload decode by valentinewallace · Pull Request #1659 · lightningdevkit/rust-lightning · GitHub
Skip to content
Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

valentinewallace
merged 3 commits into

from

Aug 15, 2022
 Merged


Fix payment onion payload decode #1659
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
dcef41d

Minor msgs::OnionHopData cleanups


valentinewallace Aug 6, 2022
a23681f

Fix bug in onion payment payload decode


valentinewallace Aug 6, 2022
dfbebbf

Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize


valentinewallace Aug 7, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


87 changes: 59 additions & 28 deletions
lightning/src/ln/msgs.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
	}
}

impl Readable for FinalOnionHopData {
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let secret: [u8; 32] = Readable::read(r)?;
 let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
 let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
 Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
	}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
 },
 OnionHopDataFormat::NonFinalNode { short_channel_id } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 },
 OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (8, payment_data, option),
 (5482373484, keysend_preimage, option)
 });
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Readable for OnionHopData {
	fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
 use bitcoin::consensus::encode::{Decodable, Error, VarInt};
 let v: VarInt = Decodable::consensus_decode(&mut r)
 .map_err(|e| match e {
 Error::Io(ioe) => DecodeError::from(ioe),
 _ => DecodeError::InvalidValue
 })?;
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let b: BigSize = Readable::read(r)?;
 const LEGACY_ONION_HOP_FLAG: u64 = 0;
 let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, v.0);
 let mut amt = HighZeroBytesDroppedVarInt(0u64);
 let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
 let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, b.0);
 let mut amt = HighZeroBytesDroppedBigSize(0u64);
 let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 let mut short_id: Option<u64> = None;
 let mut payment_data: Option<FinalOnionHopData> = None;
 let mut keysend_preimage: Option<PaymentPreimage> = None;
 // The TLV type is chosen to be compatible with lnd and c-lightning.
 decode_tlv_stream!(&mut rd, {
 (2, amt, required),
 (4, cltv_value, required),
 (6, short_id, option),
 (8, payment_data, option),
 // See https://github.com/lightning/blips/blob/master/blip-0003.md
 (5482373484, keysend_preimage, option)
 });
 rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Writeable for Ping {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
	use bitcoin::secp256k1::{PublicKey,SecretKey};
	use bitcoin::secp256k1::{Secp256k1, Message};

	use io::Cursor;
	use io::{self, Cursor};
	use prelude::*;
	use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
 assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
	}

	#[test]
	fn decode_onion_hop_data_len_as_bigsize() {
 // Tests that we can decode an onion payload that is >253 bytes.
 // Previously, receiving a payload of this size could've caused us to fail to decode a valid
 // payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
 // (little-endian).

 // Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
 // payload length to be encoded over multiple bytes rather than a single u8.
 let big_payload = encode_big_payload().unwrap();
 let mut rd = Cursor::new(&big_payload[..]);
 <msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
	}
	// see above test, needs to be a separate method for use of the serialization macros.
	fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
 use util::ser::HighZeroBytesDroppedBigSize;
 let payload = msgs::OnionHopData {
 format: OnionHopDataFormat::NonFinalNode {
 short_channel_id: 0xdeadbeef1bad1dea,
 },
 amt_to_forward: 1000,
 outgoing_cltv_value: 0xffffffff,
 };
 let mut encoded_payload = Vec::new();
 let test_bytes = vec![42u8; 1000];
 if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
 encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
 (1, test_bytes, vec_type),
 (2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 }
 Ok(encoded_payload)
	}
}

10 changes: 5 additions & 5 deletions
lightning/src/util/ser.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
	($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
 writer.write_all(&self.to_be_bytes())
 }
 }
 impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 // Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
 Ok(<$val_type>::from_be_bytes(buf))
 }
 }
 impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
 // We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
 // the high bytes being dropped. To do so, we start reading into the middle of buf
 // and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
 let first_byte = $len - ($len - total_read_len);
 let mut bytes = [0; $len];
 bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
 Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
 Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
 } else {
 // If the encoding had extra zero bytes, return a failure even though we know
 // what they meant (as the TLV test vectors require this)
Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/util/ser_macros.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
	use io::{self, Cursor};
	use prelude::*;
	use ln::msgs::DecodeError;
	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
	use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
	use bitcoin::secp256k1::PublicKey;

	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
	}

	// BOLT TLV test cases
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 let mut s = Cursor::new(s);
 let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
 let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
 let mut tlv2: Option<u64> = None;
 let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
 assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 Ok(())
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix payment onion payload decode by valentinewallace · Pull Request #1659 · lightningdevkit/rust-lightning · GitHub
Skip to content
Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

valentinewallace
merged 3 commits into

from

Aug 15, 2022
 Merged


Fix payment onion payload decode #1659
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
dcef41d

Minor msgs::OnionHopData cleanups


valentinewallace Aug 6, 2022
a23681f

Fix bug in onion payment payload decode


valentinewallace Aug 6, 2022
dfbebbf

Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize


valentinewallace Aug 7, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


87 changes: 59 additions & 28 deletions
lightning/src/ln/msgs.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
	}
}

impl Readable for FinalOnionHopData {
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let secret: [u8; 32] = Readable::read(r)?;
 let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
 let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
 Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
	}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
 },
 OnionHopDataFormat::NonFinalNode { short_channel_id } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 },
 OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (8, payment_data, option),
 (5482373484, keysend_preimage, option)
 });
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Readable for OnionHopData {
	fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
 use bitcoin::consensus::encode::{Decodable, Error, VarInt};
 let v: VarInt = Decodable::consensus_decode(&mut r)
 .map_err(|e| match e {
 Error::Io(ioe) => DecodeError::from(ioe),
 _ => DecodeError::InvalidValue
 })?;
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let b: BigSize = Readable::read(r)?;
 const LEGACY_ONION_HOP_FLAG: u64 = 0;
 let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, v.0);
 let mut amt = HighZeroBytesDroppedVarInt(0u64);
 let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
 let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, b.0);
 let mut amt = HighZeroBytesDroppedBigSize(0u64);
 let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 let mut short_id: Option<u64> = None;
 let mut payment_data: Option<FinalOnionHopData> = None;
 let mut keysend_preimage: Option<PaymentPreimage> = None;
 // The TLV type is chosen to be compatible with lnd and c-lightning.
 decode_tlv_stream!(&mut rd, {
 (2, amt, required),
 (4, cltv_value, required),
 (6, short_id, option),
 (8, payment_data, option),
 // See https://github.com/lightning/blips/blob/master/blip-0003.md
 (5482373484, keysend_preimage, option)
 });
 rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Writeable for Ping {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
	use bitcoin::secp256k1::{PublicKey,SecretKey};
	use bitcoin::secp256k1::{Secp256k1, Message};

	use io::Cursor;
	use io::{self, Cursor};
	use prelude::*;
	use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
 assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
	}

	#[test]
	fn decode_onion_hop_data_len_as_bigsize() {
 // Tests that we can decode an onion payload that is >253 bytes.
 // Previously, receiving a payload of this size could've caused us to fail to decode a valid
 // payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
 // (little-endian).

 // Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
 // payload length to be encoded over multiple bytes rather than a single u8.
 let big_payload = encode_big_payload().unwrap();
 let mut rd = Cursor::new(&big_payload[..]);
 <msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
	}
	// see above test, needs to be a separate method for use of the serialization macros.
	fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
 use util::ser::HighZeroBytesDroppedBigSize;
 let payload = msgs::OnionHopData {
 format: OnionHopDataFormat::NonFinalNode {
 short_channel_id: 0xdeadbeef1bad1dea,
 },
 amt_to_forward: 1000,
 outgoing_cltv_value: 0xffffffff,
 };
 let mut encoded_payload = Vec::new();
 let test_bytes = vec![42u8; 1000];
 if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
 encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
 (1, test_bytes, vec_type),
 (2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 }
 Ok(encoded_payload)
	}
}

10 changes: 5 additions & 5 deletions
lightning/src/util/ser.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
	($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
 writer.write_all(&self.to_be_bytes())
 }
 }
 impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 // Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
 Ok(<$val_type>::from_be_bytes(buf))
 }
 }
 impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
 // We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
 // the high bytes being dropped. To do so, we start reading into the middle of buf
 // and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
 let first_byte = $len - ($len - total_read_len);
 let mut bytes = [0; $len];
 bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
 Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
 Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
 } else {
 // If the encoding had extra zero bytes, return a failure even though we know
 // what they meant (as the TLV test vectors require this)
Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/util/ser_macros.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
	use io::{self, Cursor};
	use prelude::*;
	use ln::msgs::DecodeError;
	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
	use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
	use bitcoin::secp256k1::PublicKey;

	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
	}

	// BOLT TLV test cases
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 let mut s = Cursor::new(s);
 let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
 let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
 let mut tlv2: Option<u64> = None;
 let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
 assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 Ok(())
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Fix payment onion payload decode by valentinewallace · Pull Request #1659 · lightningdevkit/rust-lightning · GitHub
Skip to content
Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

valentinewallace
merged 3 commits into

from

Aug 15, 2022
 Merged


Fix payment onion payload decode #1659
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
dcef41d

Minor msgs::OnionHopData cleanups


valentinewallace Aug 6, 2022
a23681f

Fix bug in onion payment payload decode


valentinewallace Aug 6, 2022
dfbebbf

Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize


valentinewallace Aug 7, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


87 changes: 59 additions & 28 deletions
lightning/src/ln/msgs.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
	}
}

impl Readable for FinalOnionHopData {
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let secret: [u8; 32] = Readable::read(r)?;
 let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
 let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
 Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
	}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
 },
 OnionHopDataFormat::NonFinalNode { short_channel_id } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 },
 OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (8, payment_data, option),
 (5482373484, keysend_preimage, option)
 });
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Readable for OnionHopData {
	fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
 use bitcoin::consensus::encode::{Decodable, Error, VarInt};
 let v: VarInt = Decodable::consensus_decode(&mut r)
 .map_err(|e| match e {
 Error::Io(ioe) => DecodeError::from(ioe),
 _ => DecodeError::InvalidValue
 })?;
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let b: BigSize = Readable::read(r)?;
 const LEGACY_ONION_HOP_FLAG: u64 = 0;
 let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, v.0);
 let mut amt = HighZeroBytesDroppedVarInt(0u64);
 let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
 let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, b.0);
 let mut amt = HighZeroBytesDroppedBigSize(0u64);
 let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 let mut short_id: Option<u64> = None;
 let mut payment_data: Option<FinalOnionHopData> = None;
 let mut keysend_preimage: Option<PaymentPreimage> = None;
 // The TLV type is chosen to be compatible with lnd and c-lightning.
 decode_tlv_stream!(&mut rd, {
 (2, amt, required),
 (4, cltv_value, required),
 (6, short_id, option),
 (8, payment_data, option),
 // See https://github.com/lightning/blips/blob/master/blip-0003.md
 (5482373484, keysend_preimage, option)
 });
 rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Writeable for Ping {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
	use bitcoin::secp256k1::{PublicKey,SecretKey};
	use bitcoin::secp256k1::{Secp256k1, Message};

	use io::Cursor;
	use io::{self, Cursor};
	use prelude::*;
	use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
 assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
	}

	#[test]
	fn decode_onion_hop_data_len_as_bigsize() {
 // Tests that we can decode an onion payload that is >253 bytes.
 // Previously, receiving a payload of this size could've caused us to fail to decode a valid
 // payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
 // (little-endian).

 // Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
 // payload length to be encoded over multiple bytes rather than a single u8.
 let big_payload = encode_big_payload().unwrap();
 let mut rd = Cursor::new(&big_payload[..]);
 <msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
	}
	// see above test, needs to be a separate method for use of the serialization macros.
	fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
 use util::ser::HighZeroBytesDroppedBigSize;
 let payload = msgs::OnionHopData {
 format: OnionHopDataFormat::NonFinalNode {
 short_channel_id: 0xdeadbeef1bad1dea,
 },
 amt_to_forward: 1000,
 outgoing_cltv_value: 0xffffffff,
 };
 let mut encoded_payload = Vec::new();
 let test_bytes = vec![42u8; 1000];
 if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
 encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
 (1, test_bytes, vec_type),
 (2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 }
 Ok(encoded_payload)
	}
}

10 changes: 5 additions & 5 deletions
lightning/src/util/ser.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
	($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
 writer.write_all(&self.to_be_bytes())
 }
 }
 impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 // Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
 Ok(<$val_type>::from_be_bytes(buf))
 }
 }
 impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
 // We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
 // the high bytes being dropped. To do so, we start reading into the middle of buf
 // and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
 let first_byte = $len - ($len - total_read_len);
 let mut bytes = [0; $len];
 bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
 Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
 Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
 } else {
 // If the encoding had extra zero bytes, return a failure even though we know
 // what they meant (as the TLV test vectors require this)
Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/util/ser_macros.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
	use io::{self, Cursor};
	use prelude::*;
	use ln::msgs::DecodeError;
	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
	use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
	use bitcoin::secp256k1::PublicKey;

	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
	}

	// BOLT TLV test cases
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 let mut s = Cursor::new(s);
 let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
 let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
 let mut tlv2: Option<u64> = None;
 let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
 assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 Ok(())
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix payment onion payload decode by valentinewallace · Pull Request #1659 · lightningdevkit/rust-lightning · GitHub
Skip to content
Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

valentinewallace
merged 3 commits into

from

Aug 15, 2022
 Merged


Fix payment onion payload decode #1659
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
dcef41d

Minor msgs::OnionHopData cleanups


valentinewallace Aug 6, 2022
a23681f

Fix bug in onion payment payload decode


valentinewallace Aug 6, 2022
dfbebbf

Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize


valentinewallace Aug 7, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


87 changes: 59 additions & 28 deletions
lightning/src/ln/msgs.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
	}
}

impl Readable for FinalOnionHopData {
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let secret: [u8; 32] = Readable::read(r)?;
 let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
 let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
 Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
	}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
 },
 OnionHopDataFormat::NonFinalNode { short_channel_id } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 },
 OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (8, payment_data, option),
 (5482373484, keysend_preimage, option)
 });
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Readable for OnionHopData {
	fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
 use bitcoin::consensus::encode::{Decodable, Error, VarInt};
 let v: VarInt = Decodable::consensus_decode(&mut r)
 .map_err(|e| match e {
 Error::Io(ioe) => DecodeError::from(ioe),
 _ => DecodeError::InvalidValue
 })?;
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let b: BigSize = Readable::read(r)?;
 const LEGACY_ONION_HOP_FLAG: u64 = 0;
 let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, v.0);
 let mut amt = HighZeroBytesDroppedVarInt(0u64);
 let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
 let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, b.0);
 let mut amt = HighZeroBytesDroppedBigSize(0u64);
 let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 let mut short_id: Option<u64> = None;
 let mut payment_data: Option<FinalOnionHopData> = None;
 let mut keysend_preimage: Option<PaymentPreimage> = None;
 // The TLV type is chosen to be compatible with lnd and c-lightning.
 decode_tlv_stream!(&mut rd, {
 (2, amt, required),
 (4, cltv_value, required),
 (6, short_id, option),
 (8, payment_data, option),
 // See https://github.com/lightning/blips/blob/master/blip-0003.md
 (5482373484, keysend_preimage, option)
 });
 rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Writeable for Ping {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
	use bitcoin::secp256k1::{PublicKey,SecretKey};
	use bitcoin::secp256k1::{Secp256k1, Message};

	use io::Cursor;
	use io::{self, Cursor};
	use prelude::*;
	use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
 assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
	}

	#[test]
	fn decode_onion_hop_data_len_as_bigsize() {
 // Tests that we can decode an onion payload that is >253 bytes.
 // Previously, receiving a payload of this size could've caused us to fail to decode a valid
 // payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
 // (little-endian).

 // Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
 // payload length to be encoded over multiple bytes rather than a single u8.
 let big_payload = encode_big_payload().unwrap();
 let mut rd = Cursor::new(&big_payload[..]);
 <msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
	}
	// see above test, needs to be a separate method for use of the serialization macros.
	fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
 use util::ser::HighZeroBytesDroppedBigSize;
 let payload = msgs::OnionHopData {
 format: OnionHopDataFormat::NonFinalNode {
 short_channel_id: 0xdeadbeef1bad1dea,
 },
 amt_to_forward: 1000,
 outgoing_cltv_value: 0xffffffff,
 };
 let mut encoded_payload = Vec::new();
 let test_bytes = vec![42u8; 1000];
 if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
 encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
 (1, test_bytes, vec_type),
 (2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 }
 Ok(encoded_payload)
	}
}

10 changes: 5 additions & 5 deletions
lightning/src/util/ser.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
	($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
 writer.write_all(&self.to_be_bytes())
 }
 }
 impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 // Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
 Ok(<$val_type>::from_be_bytes(buf))
 }
 }
 impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
 // We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
 // the high bytes being dropped. To do so, we start reading into the middle of buf
 // and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
 let first_byte = $len - ($len - total_read_len);
 let mut bytes = [0; $len];
 bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
 Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
 Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
 } else {
 // If the encoding had extra zero bytes, return a failure even though we know
 // what they meant (as the TLV test vectors require this)
Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/util/ser_macros.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
	use io::{self, Cursor};
	use prelude::*;
	use ln::msgs::DecodeError;
	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
	use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
	use bitcoin::secp256k1::PublicKey;

	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
	}

	// BOLT TLV test cases
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 let mut s = Cursor::new(s);
 let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
 let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
 let mut tlv2: Option<u64> = None;
 let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
 assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 Ok(())
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
})();
Fix payment onion payload decode by valentinewallace · Pull Request #1659 · lightningdevkit/rust-lightning · GitHub
Skip to content
Fix payment onion payload decode #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

valentinewallace
merged 3 commits into

from

Aug 15, 2022
 Merged


Fix payment onion payload decode #1659
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
dcef41d

Minor msgs::OnionHopData cleanups


valentinewallace Aug 6, 2022
a23681f

Fix bug in onion payment payload decode


valentinewallace Aug 6, 2022
dfbebbf

Rename HighZeroBytesDroppedVarInt to HighZeroBytesDroppedBigSize


valentinewallace Aug 7, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


87 changes: 59 additions & 28 deletions
lightning/src/ln/msgs.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -42,7 +42,7 @@ use io_extras::read_to_end;

use util::events::MessageSendEventsProvider;
use util::logger;
use util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedVarInt, Hostname};
use util::ser::{BigSize, LengthReadable, Readable, ReadableArgs, Writeable, Writer, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname};

use ln::{PaymentPreimage, PaymentHash, PaymentSecret};

Expand DownExpand Up@@ -1375,14 +1375,14 @@ impl Writeable for OnionMessage {
impl Writeable for FinalOnionHopData {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.payment_secret.0.write(w)?;
HighZeroBytesDroppedVarInt(self.total_msat).write(w)
HighZeroBytesDroppedBigSize(self.total_msat).write(w)
	}
}

impl Readable for FinalOnionHopData {
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let secret: [u8; 32] = Readable::read(r)?;
 let amt: HighZeroBytesDroppedVarInt<u64> = Readable::read(r)?;
 let amt: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
 Ok(Self { payment_secret: PaymentSecret(secret), total_msat: amt.0 })
	}
}
Expand All@@ -1399,15 +1399,15 @@ impl Writeable for OnionHopData {
 },
 OnionHopDataFormat::NonFinalNode { short_channel_id } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 },
 OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
 encode_varint_length_prefixed_tlv!(w, {
 (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),
 (2, HighZeroBytesDroppedBigSize(self.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(self.outgoing_cltv_value), required),
 (8, payment_data, option),
 (5482373484, keysend_preimage, option)
 });
Expand All@@ -1417,36 +1417,23 @@ impl Writeable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Readable for OnionHopData {
	fn read<R: Read>(mut r: &mut R) -> Result<Self, DecodeError> {
 use bitcoin::consensus::encode::{Decodable, Error, VarInt};
 let v: VarInt = Decodable::consensus_decode(&mut r)
 .map_err(|e| match e {
 Error::Io(ioe) => DecodeError::from(ioe),
 _ => DecodeError::InvalidValue
 })?;
	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 let b: BigSize = Readable::read(r)?;
 const LEGACY_ONION_HOP_FLAG: u64 = 0;
 let (format, amt, cltv_value) = if v.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, v.0);
 let mut amt = HighZeroBytesDroppedVarInt(0u64);
 let mut cltv_value = HighZeroBytesDroppedVarInt(0u32);
 let (format, amt, cltv_value) = if b.0 != LEGACY_ONION_HOP_FLAG {
 let mut rd = FixedLengthReader::new(r, b.0);
 let mut amt = HighZeroBytesDroppedBigSize(0u64);
 let mut cltv_value = HighZeroBytesDroppedBigSize(0u32);
 let mut short_id: Option<u64> = None;
 let mut payment_data: Option<FinalOnionHopData> = None;
 let mut keysend_preimage: Option<PaymentPreimage> = None;
 // The TLV type is chosen to be compatible with lnd and c-lightning.
 decode_tlv_stream!(&mut rd, {
 (2, amt, required),
 (4, cltv_value, required),
 (6, short_id, option),
 (8, payment_data, option),
 // See https://github.com/lightning/blips/blob/master/blip-0003.md
 (5482373484, keysend_preimage, option)
 });
 rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
Expand DownExpand Up@@ -1488,6 +1475,14 @@ impl Readable for OnionHopData {
	}
}

// ReadableArgs because we need onion_utils::decode_next_hop to accommodate payment packets and
// onion message packets.
impl ReadableArgs<()> for OnionHopData {
	fn read<R: Read>(r: &mut R, _arg: ()) -> Result<Self, DecodeError> {
 <Self as Readable>::read(r)
	}
}

impl Writeable for Ping {
	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 self.ponglen.write(w)?;
Expand DownExpand Up@@ -1913,7 +1908,7 @@ mod tests {
	use bitcoin::secp256k1::{PublicKey,SecretKey};
	use bitcoin::secp256k1::{Secp256k1, Message};

	use io::Cursor;
	use io::{self, Cursor};
	use prelude::*;
	use core::convert::TryFrom;

Expand DownExpand Up@@ -2824,4 +2819,40 @@ mod tests {
 assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000);
 assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff);
	}

	#[test]
	fn decode_onion_hop_data_len_as_bigsize() {
 // Tests that we can decode an onion payload that is >253 bytes.
 // Previously, receiving a payload of this size could've caused us to fail to decode a valid
 // payload, because we were decoding the length (a BigSize, big-endian) as a VarInt
 // (little-endian).

 // Encode a test onion payload with a big custom TLV such that it's >253 bytes, forcing the
 // payload length to be encoded over multiple bytes rather than a single u8.
 let big_payload = encode_big_payload().unwrap();
 let mut rd = Cursor::new(&big_payload[..]);
 <msgs::OnionHopData as Readable>::read(&mut rd).unwrap();
	}
	// see above test, needs to be a separate method for use of the serialization macros.
	fn encode_big_payload() -> Result<Vec<u8>, io::Error> {
 use util::ser::HighZeroBytesDroppedBigSize;
 let payload = msgs::OnionHopData {
 format: OnionHopDataFormat::NonFinalNode {
 short_channel_id: 0xdeadbeef1bad1dea,
 },
 amt_to_forward: 1000,
 outgoing_cltv_value: 0xffffffff,
 };
 let mut encoded_payload = Vec::new();
 let test_bytes = vec![42u8; 1000];
 if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
 encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
 (1, test_bytes, vec_type),
 (2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
 (4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
 (6, short_channel_id, required)
 });
 }
 Ok(encoded_payload)
	}
}

10 changes: 5 additions & 5 deletions
lightning/src/util/ser.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -400,7 +400,7 @@ impl Readable for BigSize {
/// variable-length integer which is simply truncated by skipping high zero bytes. This type
/// encapsulates such integers implementing Readable/Writeable for them.
#[cfg_attr(test, derive(PartialEq, Debug))]
pub(crate) struct HighZeroBytesDroppedVarInt<T>(pub T);
pub(crate) struct HighZeroBytesDroppedBigSize<T>(pub T);

macro_rules! impl_writeable_primitive {
	($val_type:ty, $len: expr) => {
Expand All@@ -410,7 +410,7 @@ macro_rules! impl_writeable_primitive {
 writer.write_all(&self.to_be_bytes())
 }
 }
 impl Writeable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Writeable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 // Skip any full leading 0 bytes when writing (in BE):
Expand All@@ -425,9 +425,9 @@ macro_rules! impl_writeable_primitive {
 Ok(<$val_type>::from_be_bytes(buf))
 }
 }
 impl Readable for HighZeroBytesDroppedVarInt<$val_type> {
 impl Readable for HighZeroBytesDroppedBigSize<$val_type> {
 #[inline]
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedVarInt<$val_type>, DecodeError> {
 fn read<R: Read>(reader: &mut R) -> Result<HighZeroBytesDroppedBigSize<$val_type>, DecodeError> {
 // We need to accept short reads (read_len == 0) as "EOF" and handle them as simply
 // the high bytes being dropped. To do so, we start reading into the middle of buf
 // and then convert the appropriate number of bytes with extra high bytes out of
Expand All@@ -443,7 +443,7 @@ macro_rules! impl_writeable_primitive {
 let first_byte = $len - ($len - total_read_len);
 let mut bytes = [0; $len];
 bytes.copy_from_slice(&buf[first_byte..first_byte + $len]);
 Ok(HighZeroBytesDroppedVarInt(<$val_type>::from_be_bytes(bytes)))
 Ok(HighZeroBytesDroppedBigSize(<$val_type>::from_be_bytes(bytes)))
 } else {
 // If the encoding had extra zero bytes, return a failure even though we know
 // what they meant (as the TLV test vectors require this)
Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/util/ser_macros.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -563,7 +563,7 @@ mod tests {
	use io::{self, Cursor};
	use prelude::*;
	use ln::msgs::DecodeError;
	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
	use util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter};
	use bitcoin::secp256k1::PublicKey;

	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
Expand DownExpand Up@@ -632,9 +632,9 @@ mod tests {
	}

	// BOLT TLV test cases
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 let mut s = Cursor::new(s);
 let mut tlv1: Option<HighZeroBytesDroppedVarInt<u64>> = None;
 let mut tlv1: Option<HighZeroBytesDroppedBigSize<u64>> = None;
 let mut tlv2: Option<u64> = None;
 let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 let mut tlv4: Option<u16> = None;
Expand DownExpand Up@@ -765,11 +765,11 @@ mod tests {
 assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 stream.0.clear();
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)});
 assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());

 Ok(())
Expand Down