Skip to content

Commit 60fde9e

Browse files
committed
Check that we only accept INPUT_CHARSET in from_str
This results in fuzzer crash when trying compute the checksum in `Display` implemenation of Descriptor
1 parent a396605 commit 60fde9e

File tree

5 files changed

+23
-23
lines changed

5 files changed

+23
-23
lines changed

src/descriptor/checksum.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
use core::fmt;
99
use core::iter::FromIterator;
1010

11+
pub use crate::expression::INPUT_CHARSET;
1112
use crate::prelude::*;
1213
use crate::Error;
1314

14-
const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
1515
const CHECKSUM_CHARSET: &[u8] = b"qpzry9x8gf2tvdw0s3jn54khce6mua7l";
1616

1717
fn poly_mod(mut c: u64, val: u64) -> u64 {

src/descriptor/tr.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,11 +518,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Tr<Pk> {
518518

519519
// Helper function to parse string into miniscript tree form
520520
fn parse_tr_tree(s: &str) -> Result<expression::Tree, Error> {
521-
for ch in s.bytes() {
522-
if !ch.is_ascii() {
523-
return Err(Error::Unprintable(ch));
524-
}
525-
}
521+
expression::check_valid_chars(s)?;
526522

527523
if s.len() > 3 && &s[..3] == "tr(" && s.as_bytes()[s.len() - 1] == b')' {
528524
let rest = &s[3..s.len() - 1];

src/expression.rs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ use core::str::FromStr;
88
use crate::prelude::*;
99
use crate::{errstr, Error, MAX_RECURSION_DEPTH};
1010

11+
/// Allowed characters are descriptor strings.
12+
pub const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
13+
1114
#[derive(Debug)]
1215
/// A token of the form `x(...)` or `x`
1316
pub struct Tree<'a> {
@@ -166,13 +169,7 @@ impl<'a> Tree<'a> {
166169
/// Parses a tree from a string
167170
#[allow(clippy::should_implement_trait)] // Cannot use std::str::FromStr because of lifetimes.
168171
pub fn from_str(s: &'a str) -> Result<Tree<'a>, Error> {
169-
// Filter out non-ASCII because we byte-index strings all over the
170-
// place and Rust gets very upset when you splinch a string.
171-
for ch in s.bytes() {
172-
if !ch.is_ascii() {
173-
return Err(Error::Unprintable(ch));
174-
}
175-
}
172+
check_valid_chars(s)?;
176173

177174
let (top, rem) = Tree::from_slice(s)?;
178175
if rem.is_empty() {
@@ -183,6 +180,21 @@ impl<'a> Tree<'a> {
183180
}
184181
}
185182

183+
/// Filter out non-ASCII because we byte-index strings all over the
184+
/// place and Rust gets very upset when you splinch a string.
185+
pub fn check_valid_chars(s: &str) -> Result<(), Error> {
186+
for ch in s.bytes() {
187+
if !ch.is_ascii() {
188+
return Err(Error::Unprintable(ch));
189+
}
190+
// TODO: Avoid linear search overhead by using OnceCell to cache this in a BTreeMap.
191+
INPUT_CHARSET
192+
.find(char::from(ch))
193+
.ok_or_else(|| Error::Unprintable(ch))?;
194+
}
195+
Ok(())
196+
}
197+
186198
/// Parse a string as a u32, for timelocks or thresholds
187199
pub fn parse_num(s: &str) -> Result<u32, Error> {
188200
if s.len() > 1 {

src/policy/concrete.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,11 +1061,7 @@ impl_from_str!(
10611061
Policy<Pk>,
10621062
type Err = Error;,
10631063
fn from_str(s: &str) -> Result<Policy<Pk>, Error> {
1064-
for ch in s.as_bytes() {
1065-
if *ch < 20 || *ch > 127 {
1066-
return Err(Error::Unprintable(*ch));
1067-
}
1068-
}
1064+
expression::check_valid_chars(s)?;
10691065

10701066
let tree = expression::Tree::from_str(s)?;
10711067
let policy: Policy<Pk> = FromTree::from_tree(&tree)?;

src/policy/semantic.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -315,11 +315,7 @@ impl_from_str!(
315315
Policy<Pk>,
316316
type Err = Error;,
317317
fn from_str(s: &str) -> Result<Policy<Pk>, Error> {
318-
for ch in s.as_bytes() {
319-
if *ch < 20 || *ch > 127 {
320-
return Err(Error::Unprintable(*ch));
321-
}
322-
}
318+
expression::check_valid_chars(s)?;
323319

324320
let tree = expression::Tree::from_str(s)?;
325321
expression::FromTree::from_tree(&tree)

0 commit comments

Comments
 (0)