Skip to content

Commit

Permalink
Enforce maximum string length
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tcharding
tcharding committed Oct 22, 2023
1 parent a0fa43d commit 65790bf
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 25 deletions.
113 changes: 113 additions & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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>(
Expand All @@ -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>(
Expand All @@ -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>(
Expand Down Expand Up @@ -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());
}
}
21 changes: 17 additions & 4 deletions src/primitives/decode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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)`].
Expand Down Expand Up @@ -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
///
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
Expand All @@ -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),
}
Expand All @@ -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),
}
Expand Down
5 changes: 5 additions & 0 deletions src/primitives/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
Loading

0 comments on commit 65790bf

Please sign in to comment.