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.
@@ -166,7 +171,10 @@ pub use {
 /// If this function succeeds the input string was found to be well formed (hrp, separator, bech32
 /// characters), and to have either a valid bech32m checksum or a valid bech32 checksum.
 ///
-/// If your input string has no checksum use the [`CheckedHrpstring`] constructor, which allows selecting the checksum algorithm explicitly.
+/// 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
 ///
@@ -213,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> {
@@ -223,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> {
@@ -235,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> {
@@ -247,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,
@@ -260,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,
@@ -278,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,
@@ -296,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>(
@@ -310,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>(
@@ -329,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>(
@@ -492,4 +581,22 @@ 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());
+    }
 }

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).
@@ -127,6 +128,11 @@ impl<'s> UncheckedHrpstring<'s> {
     /// Checks for valid ASCII values, does not validate the checksum.
     #[inline]
     pub fn new(s: &'s str) -> Result<Self, UncheckedHrpstringError> {
+        let len = s.len();
+        if len > MAX_STRING_LENGTH {
+            return Err(UncheckedHrpstringError::TooLong(len));
+        }
+
         let sep_pos = check_characters(s)?;
         let (hrp, data) = s.split_at(sep_pos);
 
@@ -658,6 +664,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 +677,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 +691,7 @@ impl std::error::Error for UncheckedHrpstringError {
         use UncheckedHrpstringError::*;
 
         match *self {
+            TooLong(_) => None,
             Char(ref e) => Some(e),
             Hrp(ref e) => Some(e),
         }
@@ -827,7 +838,7 @@ mod tests {
             ("\u{80}1eym55h",
              Hrp(hrp::Error::NonAsciiChar('\u{80}'))),
             ("an84characterslonghumanreadablepartthatcontainsthetheexcludedcharactersbioandnumber11d6pts4",
-             Hrp(hrp::Error::TooLong(84))),
+             TooLong(91)),
             ("pzry9x0s0muk",
              Char(CharError::MissingSeparator)),
             ("1pzry9x0s0muk",
@@ -880,7 +891,7 @@ mod tests {
             ("\u{80}1g6xzxy",
              Hrp(hrp::Error::NonAsciiChar('\u{80}'))),
             ("an84characterslonghumanreadablepartthatcontainsthenumber1andtheexcludedcharactersbio1569pvx",
-             Hrp(hrp::Error::TooLong(84))),
+             TooLong(91)),
             ("qyrz8wqd2c9m",
              Char(CharError::MissingSeparator)),
             ("1qyrz8wqd2c9m",

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 {}