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

Backport of 60fde9e from #569.

Author: apoelstra

fuzz/fuzz_targets/roundtrip_descriptor.rs

@@ -26,6 +26,6 @@ mod tests {
     use super::*;
     #[test]
     fn test() {
-        do_test(b"pkh()");
+        do_test(b"tr(}4872r1111r(r)");
     }
 }

src/descriptor/checksum.rs

@@ -8,10 +8,10 @@
 use core::fmt;
 use core::iter::FromIterator;
 
+pub use crate::expression::INPUT_CHARSET;
 use crate::prelude::*;
 use crate::Error;
 
-const INPUT_CHARSET: &str =  "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
 const CHECKSUM_CHARSET: &[u8] = b"qpzry9x8gf2tvdw0s3jn54khce6mua7l";
 
 fn poly_mod(mut c: u64, val: u64) -> u64 {

src/descriptor/tr.rs

@@ -514,11 +514,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Tr<Pk> {
 
 // Helper function to parse string into miniscript tree form
 fn parse_tr_tree(s: &str) -> Result<expression::Tree<'_>, Error> {
-    for ch in s.bytes() {
-        if !ch.is_ascii() {
-            return Err(Error::Unprintable(ch));
-        }
-    }
+    expression::check_valid_chars(s)?;
 
     if s.len() > 3 && &s[..3] == "tr(" && s.as_bytes()[s.len() - 1] == b')' {
         let rest = &s[3..s.len() - 1];

src/expression.rs

@@ -8,6 +8,9 @@ use core::str::FromStr;
 use crate::prelude::*;
 use crate::{errstr, Error, MAX_RECURSION_DEPTH};
 
+/// Allowed characters are descriptor strings.
+pub const INPUT_CHARSET: &str =  "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
+
 #[derive(Debug)]
 /// A token of the form `x(...)` or `x`
 pub struct Tree<'a> {
@@ -166,13 +169,7 @@ impl<'a> Tree<'a> {
     /// Parses a tree from a string
     #[allow(clippy::should_implement_trait)] // Cannot use std::str::FromStr because of lifetimes.
     pub fn from_str(s: &'a str) -> Result<Tree<'a>, Error> {
-        // Filter out non-ASCII because we byte-index strings all over the
-        // place and Rust gets very upset when you splinch a string.
-        for ch in s.bytes() {
-            if !ch.is_ascii() {
-                return Err(Error::Unprintable(ch));
-            }
-        }
+        check_valid_chars(s)?;
 
         let (top, rem) = Tree::from_slice(s)?;
         if rem.is_empty() {
@@ -183,6 +180,21 @@ impl<'a> Tree<'a> {
     }
 }
 
+/// Filter out non-ASCII because we byte-index strings all over the
+/// place and Rust gets very upset when you splinch a string.
+pub fn check_valid_chars(s: &str) -> Result<(), Error> {
+    for ch in s.bytes() {
+        if !ch.is_ascii() {
+            return Err(Error::Unprintable(ch));
+        }
+        // TODO: Avoid linear search overhead by using OnceCell to cache this in a BTreeMap.
+        INPUT_CHARSET
+            .find(char::from(ch))
+            .ok_or_else(|| Error::Unprintable(ch))?;
+    }
+    Ok(())
+}
+
 /// Parse a string as a u32, for timelocks or thresholds
 pub fn parse_num(s: &str) -> Result<u32, Error> {
     if s.len() > 1 {

src/policy/concrete.rs

@@ -1101,11 +1101,7 @@ impl_from_str!(
     Policy<Pk>,
     type Err = Error;,
     fn from_str(s: &str) -> Result<Policy<Pk>, Error> {
-        for ch in s.as_bytes() {
-            if *ch < 20 || *ch > 127 {
-                return Err(Error::Unprintable(*ch));
-            }
-        }
+        expression::check_valid_chars(s)?;
 
         let tree = expression::Tree::from_str(s)?;
         let policy: Policy<Pk> = FromTree::from_tree(&tree)?;

src/policy/semantic.rs

@@ -310,11 +310,7 @@ impl_from_str!(
     Policy<Pk>,
     type Err = Error;,
     fn from_str(s: &str) -> Result<Policy<Pk>, Error> {
-        for ch in s.as_bytes() {
-            if *ch < 20 || *ch > 127 {
-                return Err(Error::Unprintable(*ch));
-            }
-        }
+        expression::check_valid_chars(s)?;
 
         let tree = expression::Tree::from_str(s)?;
         expression::FromTree::from_tree(&tree)