Enforce maximum string length

BIP-173 states that a bech32 string must not exceed 90 characters.
However BOLT-11 states that the string limit may be exceeded. This puts
us, architecturally speaking in a conundrum - we want to support
lightning but this crate pretty heavily documents itself as an
implementation of BIP-173 and BIP-350.

The solution we choose is to enforce the string limit in the segwit
modules and not in the functions in `lib.rs`. We document this decision
in the crate level docs as well as on the individual functions.

FTR in `bech32 v0.9.0` the lengths were not enforced either.

Author: tcharding

src/lib.rs

@@ -9,6 +9,9 @@
 //! a data part. A checksum at the end of the string provides error detection to prevent mistakes
 //! when the string is written off or read out loud.
 //!
+//! Please note, so as to support lighting ([BOLT-11]) we explicitly do not do string length checks
+//! in the top level API. We do however enforce the 90 character limit within the `segwit` modules.
+//!
 //! # Usage
 //!
 //! - If you are doing segwit stuff you likely want to use the [`segwit`] API.
@@ -113,6 +116,8 @@
 //!
 //! # }
 //! ```
+//!
+//! [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 
 #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
 // Experimental features we need.
@@ -169,6 +174,8 @@ pub use {
 /// If your input string has no checksum use the [`CheckedHrpstring`] constructor, which allows
 /// selecting the checksum algorithm explicitly.
 ///
+/// Note: this function does not enforce any restrictions on the total length of the input string.
+///
 /// # Returns
 ///
 /// The human-readable part and the encoded data with the checksum removed.
@@ -214,6 +221,15 @@ pub fn decode(s: &str) -> Result<(Hrp, Vec<u8>), DecodeError> {
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[cfg(feature = "alloc")]
 #[inline]
 pub fn encode<Ck: Checksum>(hrp: Hrp, data: &[u8]) -> Result<String, fmt::Error> {
@@ -224,6 +240,15 @@ pub fn encode<Ck: Checksum>(hrp: Hrp, data: &[u8]) -> Result<String, fmt::Error>
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[cfg(feature = "alloc")]
 #[inline]
 pub fn encode_lower<Ck: Checksum>(hrp: Hrp, data: &[u8]) -> Result<String, fmt::Error> {
@@ -236,6 +261,15 @@ pub fn encode_lower<Ck: Checksum>(hrp: Hrp, data: &[u8]) -> Result<String, fmt::
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[cfg(feature = "alloc")]
 #[inline]
 pub fn encode_upper<Ck: Checksum>(hrp: Hrp, data: &[u8]) -> Result<String, fmt::Error> {
@@ -248,6 +282,15 @@ pub fn encode_upper<Ck: Checksum>(hrp: Hrp, data: &[u8]) -> Result<String, fmt::
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[inline]
 pub fn encode_to_fmt<Ck: Checksum, W: fmt::Write>(
     fmt: &mut W,
@@ -261,6 +304,15 @@ pub fn encode_to_fmt<Ck: Checksum, W: fmt::Write>(
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[inline]
 pub fn encode_lower_to_fmt<Ck: Checksum, W: fmt::Write>(
     fmt: &mut W,
@@ -279,6 +331,15 @@ pub fn encode_lower_to_fmt<Ck: Checksum, W: fmt::Write>(
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[inline]
 pub fn encode_upper_to_fmt<Ck: Checksum, W: fmt::Write>(
     fmt: &mut W,
@@ -297,6 +358,15 @@ pub fn encode_upper_to_fmt<Ck: Checksum, W: fmt::Write>(
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[cfg(feature = "std")]
 #[inline]
 pub fn encode_to_writer<Ck: Checksum, W: std::io::Write>(
@@ -311,6 +381,15 @@ pub fn encode_to_writer<Ck: Checksum, W: std::io::Write>(
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[cfg(feature = "std")]
 #[inline]
 pub fn encode_lower_to_writer<Ck: Checksum, W: std::io::Write>(
@@ -330,6 +409,15 @@ pub fn encode_lower_to_writer<Ck: Checksum, W: std::io::Write>(
 ///
 /// Encoded string will be prefixed with the `hrp` and have a checksum appended as specified by the
 /// `Ck` algorithm (`NoChecksum` to exclude checksum all together).
+///
+/// ## Deviation from spec (BIP-173)
+///
+/// In order to support [BOLT-11] this function does not restrict the total length of the returned
+/// string. To encode [BIP-173] / [BIP-350] compliant segwit addresses use [`segwit::encode`].
+///
+/// [BIP-173]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+/// [BIP-350]: <https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki>
+/// [BOLT-11]: <https://github.com/lightning/bolts/blob/master/11-payment-encoding.md>
 #[cfg(feature = "std")]
 #[inline]
 pub fn encode_upper_to_writer<Ck: Checksum, W: std::io::Write>(
@@ -493,4 +581,29 @@ mod tests {
 
         assert_eq!(got, want);
     }
+
+    #[test]
+    fn can_encode_really_long_string() {
+        // Encode around the bech32 limit, mainly here as documentation.
+        let tcs = vec![
+            // Also shows there are no limit checks on the data slice (since segwit limits this to 40 bytes).
+            ([0_u8; 50], Hrp::parse_unchecked("abc")), // Encodes to 90 characters.
+            ([0_u8; 50], Hrp::parse_unchecked("abcd")), // Encodes to 91 characters.
+        ];
+        for (data, hrp) in tcs {
+            assert!(encode::<Bech32>(&hrp, &data).is_ok());
+        }
+
+        // Encode something arbitrarily long.
+        let data = [0_u8; 1024];
+        let hrp = Hrp::parse_unchecked("abc");
+        assert!(encode::<Bech32m>(&hrp, &data).is_ok());
+    }
+
+    #[test]
+    fn can_decode_long_string() {
+        // A  91 character long string, greater than the segwit enforced maximum of 90.
+        let s = "abcd1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqrw9z3s";
+        assert!(decode(s).is_ok());
+    }
 }

src/primitives/decode.rs

@@ -81,6 +81,7 @@ use crate::primitives::gf32::Fe32;
 use crate::primitives::hrp::{self, Hrp};
 use crate::primitives::iter::{Fe32IterExt, FesToBytes};
 use crate::primitives::segwit::{self, WitnessLengthError, VERSION_0};
+use crate::primitives::MAX_STRING_LENGTH;
 use crate::{Bech32, Bech32m};
 
 /// Separator between the hrp and payload (as defined by BIP-173).
@@ -89,7 +90,7 @@ const SEP: char = '1';
 /// An HRP string that has been parsed but not yet had the checksum checked.
 ///
 /// Parsing an HRP string only checks validity of the characters, it does not validate the
-/// checksum in any way.
+/// checksum in any way. Nor do we check the length of the parsed string.
 ///
 /// Unless you are attempting to validate a string with multiple checksums then you likely do not
 /// want to use this type directly, instead use [`CheckedHrpstring::new(s)`].
@@ -217,7 +218,9 @@ impl<'s> UncheckedHrpstring<'s> {
 /// > We first describe the general checksummed base32 format called Bech32 and then
 /// > define Segregated Witness addresses using it.
 ///
-/// This type abstracts over the general checksummed base32 format called Bech32.
+/// This type abstracts over the general checksummed base32 format called Bech32. In order to
+/// support `BOLT-11` we explicitly do not check the length of the parsed string, this is however,
+/// done by the [`SegwitHrpstring`] type.
 ///
 /// # Examples
 ///
@@ -331,8 +334,8 @@ impl<'s> CheckedHrpstring<'s> {
     }
 }
 
-/// An HRP string that has been parsed, had the checksum validated, had the witness version
-/// validated, had the witness data length checked, and the had witness version and checksum
+/// An valid length HRP string that has been parsed, had the checksum validated, had the witness
+/// version validated, had the witness data length checked, and the had witness version and checksum
 /// removed.
 ///
 /// # Examples
@@ -368,6 +371,11 @@ impl<'s> SegwitHrpstring<'s> {
     /// to get strict BIP conformance (also [`Hrp::is_valid_on_mainnet`] and friends).
     #[inline]
     pub fn new(s: &'s str) -> Result<Self, SegwitHrpstringError> {
+        let len = s.len();
+        if len > MAX_STRING_LENGTH {
+            return Err(SegwitHrpstringError::Unchecked(UncheckedHrpstringError::TooLong(len)));
+        }
+
         let unchecked = UncheckedHrpstring::new(s)?;
 
         if unchecked.data.is_empty() {
@@ -658,6 +666,8 @@ impl From<ChecksumError> for CheckedHrpstringError {
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub enum UncheckedHrpstringError {
+    /// String exceeds maximum allowed length.
+    TooLong(usize),
     /// An error with the characters of the input string.
     Char(CharError),
     /// The human-readable part is invalid.
@@ -669,6 +679,8 @@ impl fmt::Display for UncheckedHrpstringError {
         use UncheckedHrpstringError::*;
 
         match *self {
+            TooLong(len) =>
+                write!(f, "string exceeds spec limit of {} chars: {}", MAX_STRING_LENGTH, len),
             Char(ref e) => write_err!(f, "character error"; e),
             Hrp(ref e) => write_err!(f, "invalid human-readable part"; e),
         }
@@ -681,6 +693,7 @@ impl std::error::Error for UncheckedHrpstringError {
         use UncheckedHrpstringError::*;
 
         match *self {
+            TooLong(_) => None,
             Char(ref e) => Some(e),
             Hrp(ref e) => Some(e),
         }

src/primitives/mod.rs

@@ -12,6 +12,11 @@ pub mod segwit;
 
 use checksum::{Checksum, PackedNull};
 
+/// The maximum allowed length of a bech32 string (see [`BIP-173`]).
+///
+/// [`BIP-173`]: <https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki>
+pub const MAX_STRING_LENGTH: usize = 90;
+
 /// The "null checksum" used on bech32 strings for which we want to do no checksum checking.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum NoChecksum {}