Fix crash due to index-out-of-bounds in feature translation

[rust-lightning] / lightning / src / ln / features.rs

diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs
index dd6ac3307f88d1c14a97a7874e1de2556d87696b..9d865d15ec2bbc6f11e8ce39b2c970a0f48d4314 100644 (file)
--- a/lightning/src/ln/features.rs
+++ b/lightning/src/ln/features.rs
@@ -22,8 +22,9 @@
 //! [BOLT #9]: https://github.com/lightningnetwork/lightning-rfc/blob/master/09-features.md
 //! [messages]: crate::ln::msgs
 
-use std::{cmp, fmt};
-use std::marker::PhantomData;
+use prelude::*;
+use core::{cmp, fmt};
+use core::marker::PhantomData;
 
 use bitcoin::bech32;
 use bitcoin::bech32::{Base32Len, FromBase32, ToBase32, u5, WriteBase32};
@@ -31,6 +32,7 @@ use ln::msgs::DecodeError;
 use util::ser::{Readable, Writeable, Writer};
 
 mod sealed {
+       use prelude::*;
        use ln::features::Features;
 
        /// The context in which [`Features`] are applicable. Defines which features are required and
@@ -95,7 +97,7 @@ mod sealed {
                        // Byte 0
                        ,
                        // Byte 1
-                       StaticRemoteKey | PaymentSecret,
+                       VariableLengthOnion | StaticRemoteKey | PaymentSecret,
                        // Byte 2
                        ,
                        // Byte 3
@@ -105,7 +107,7 @@ mod sealed {
                        // Byte 0
                        DataLossProtect | InitialRoutingSync | UpfrontShutdownScript | GossipQueries,
                        // Byte 1
-                       VariableLengthOnion,
+                       ,
                        // Byte 2
                        BasicMPP,
                        // Byte 3
@@ -117,7 +119,7 @@ mod sealed {
                        // Byte 0
                        ,
                        // Byte 1
-                       StaticRemoteKey | PaymentSecret,
+                       VariableLengthOnion | StaticRemoteKey | PaymentSecret,
                        // Byte 2
                        ,
                        // Byte 3
@@ -127,7 +129,7 @@ mod sealed {
                        // Byte 0
                        DataLossProtect | UpfrontShutdownScript | GossipQueries,
                        // Byte 1
-                       VariableLengthOnion,
+                       ,
                        // Byte 2
                        BasicMPP,
                        // Byte 3
@@ -143,7 +145,7 @@ mod sealed {
                        // Byte 0
                        ,
                        // Byte 1
-                       PaymentSecret,
+                       VariableLengthOnion | PaymentSecret,
                        // Byte 2
                        ,
                ],
@@ -151,7 +153,7 @@ mod sealed {
                        // Byte 0
                        ,
                        // Byte 1
-                       VariableLengthOnion,
+                       ,
                        // Byte 2
                        BasicMPP,
                ],
@@ -488,13 +490,14 @@ impl<T: sealed::Context> Features<T> {
        /// Converts `Features<T>` to `Features<C>`. Only known `T` features relevant to context `C` are
        /// included in the result.
        fn to_context_internal<C: sealed::Context>(&self) -> Features<C> {
-               let byte_count = C::KNOWN_FEATURE_MASK.len();
+               let from_byte_count = T::KNOWN_FEATURE_MASK.len();
+               let to_byte_count = C::KNOWN_FEATURE_MASK.len();
                let mut flags = Vec::new();
                for (i, byte) in self.flags.iter().enumerate() {
-                       if i < byte_count {
-                               let known_source_features = T::KNOWN_FEATURE_MASK[i];
-                               let known_target_features = C::KNOWN_FEATURE_MASK[i];
-                               flags.push(byte & known_source_features & known_target_features);
+                       if i < from_byte_count && i < to_byte_count {
+                               let from_known_features = T::KNOWN_FEATURE_MASK[i];
+                               let to_known_features = C::KNOWN_FEATURE_MASK[i];
+                               flags.push(byte & from_known_features & to_known_features);
                        }
                }
                Features::<C> { flags, mark: PhantomData, }
@@ -502,6 +505,7 @@ impl<T: sealed::Context> Features<T> {
 
        /// Create a Features given a set of flags, in little-endian. This is in reverse byte order from
        /// most on-the-wire encodings.
+       /// (C-not exported) as we don't support export across multiple T
        pub fn from_le_bytes(flags: Vec<u8>) -> Features<T> {
                Features {
                        flags,
@@ -645,11 +649,8 @@ impl<T: sealed::PaymentSecret> Features<T> {
        pub(crate) fn requires_payment_secret(&self) -> bool {
                <T as sealed::PaymentSecret>::requires_feature(&self.flags)
        }
-       // Note that we never need to test this since what really matters is the invoice - iff the
-       // invoice provides a payment_secret, we assume that we can use it (ie that the recipient
-       // supports payment_secret).
-       #[allow(dead_code)]
-       pub(crate) fn supports_payment_secret(&self) -> bool {
+       /// Returns whether the `payment_secret` feature is supported.
+       pub fn supports_payment_secret(&self) -> bool {
                <T as sealed::PaymentSecret>::supports_feature(&self.flags)
        }
 }
@@ -729,8 +730,10 @@ mod tests {
 
                assert!(InitFeatures::known().supports_variable_length_onion());
                assert!(NodeFeatures::known().supports_variable_length_onion());
-               assert!(!InitFeatures::known().requires_variable_length_onion());
-               assert!(!NodeFeatures::known().requires_variable_length_onion());
+               assert!(InvoiceFeatures::known().supports_variable_length_onion());
+               assert!(InitFeatures::known().requires_variable_length_onion());
+               assert!(NodeFeatures::known().requires_variable_length_onion());
+               assert!(InvoiceFeatures::known().requires_variable_length_onion());
 
                assert!(InitFeatures::known().supports_static_remote_key());
                assert!(NodeFeatures::known().supports_static_remote_key());
@@ -739,13 +742,17 @@ mod tests {
 
                assert!(InitFeatures::known().supports_payment_secret());
                assert!(NodeFeatures::known().supports_payment_secret());
+               assert!(InvoiceFeatures::known().supports_payment_secret());
                assert!(InitFeatures::known().requires_payment_secret());
                assert!(NodeFeatures::known().requires_payment_secret());
+               assert!(InvoiceFeatures::known().requires_payment_secret());
 
                assert!(InitFeatures::known().supports_basic_mpp());
                assert!(NodeFeatures::known().supports_basic_mpp());
+               assert!(InvoiceFeatures::known().supports_basic_mpp());
                assert!(!InitFeatures::known().requires_basic_mpp());
                assert!(!NodeFeatures::known().requires_basic_mpp());
+               assert!(!InvoiceFeatures::known().requires_basic_mpp());
 
                assert!(InitFeatures::known().supports_shutdown_anysegwit());
                assert!(NodeFeatures::known().supports_shutdown_anysegwit());
@@ -786,12 +793,12 @@ mod tests {
                {
                        // Check that the flags are as expected:
                        // - option_data_loss_protect
-                       // - var_onion_optin | static_remote_key (req) | payment_secret(req)
+                       // - var_onion_optin (req) | static_remote_key (req) | payment_secret(req)
                        // - basic_mpp
                        // - opt_shutdown_anysegwit
                        assert_eq!(node_features.flags.len(), 4);
                        assert_eq!(node_features.flags[0], 0b00000010);
-                       assert_eq!(node_features.flags[1], 0b01010010);
+                       assert_eq!(node_features.flags[1], 0b01010001);
                        assert_eq!(node_features.flags[2], 0b00000010);
                        assert_eq!(node_features.flags[3], 0b00001000);
                }