Fix fuzz crashes (fixes #89)

* fix `thresh` and `multi` descriptors with no arguments
* fix roundtrip_concrete roundtrip for "older(03)"
* fix roundtrip_descriptor for "c:pk_k" ("pk" alias)

Author: kiminuo

fuzz/fuzz_targets/roundtrip_descriptor.rs

@@ -9,7 +9,8 @@ fn do_test(data: &[u8]) {
     let s = String::from_utf8_lossy(data);
     if let Ok(desc) = Descriptor::<DummyKey>::from_str(&s) {
         let output = desc.to_string();
-        assert_eq!(s, output);
+        let normalize_aliases = s.replace("c:pk_k(", "pk(");
+        assert_eq!(normalize_aliases, output);
     }
 }
 
@@ -58,4 +59,11 @@ mod tests {
         extend_vec_from_hex("00", &mut a);
         super::do_test(&a);
     }
+
+    #[test]
+    fn test_cpkk_alias() {
+        let mut a = Vec::new();
+        extend_vec_from_hex("633a706b5f6b2829", &mut a); // c:pk_k()
+        super::do_test(&a);
+    }
 }

fuzz/fuzz_targets/roundtrip_policy.rs

@@ -64,4 +64,11 @@ mod tests {
         extend_vec_from_hex("00", &mut a);
         super::do_test(&a);
     }
+
+    #[test]
+    fn duplicate_crash_2() {
+        let mut a = Vec::new();
+        extend_vec_from_hex("746872657368", &mut a); // thresh
+        super::do_test(&a);
+    }
 }

src/descriptor/mod.rs

@@ -896,4 +896,22 @@ mod tests {
 
         assert_eq!(check, &Instruction::Op(OP_CSV))
     }
+
+    #[test]
+    fn empty_multi() {
+        let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("multi");
+        assert_eq!(
+            descriptor.unwrap_err().to_string(),
+            "unexpected «no arguments given»"
+        )
+    }
+
+    #[test]
+    fn empty_thresh() {
+        let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("thresh");
+        assert_eq!(
+            descriptor.unwrap_err().to_string(),
+            "unexpected «no arguments given»"
+        )
+    }
 }

src/expression.rs

@@ -127,6 +127,14 @@ impl<'a> Tree<'a> {
 
 /// Parse a string as a u32, for timelocks or thresholds
 pub fn parse_num(s: &str) -> Result<u32, Error> {
+    if s.len() > 1 {
+        let ch = s.chars().next().unwrap();
+        if ch < '1' || ch > '9' {
+            return Err(Error::Unexpected(
+                "Number must start with a digit 1-9".to_string(),
+            ));
+        }
+    }
     u32::from_str(s).map_err(|_| errstr(s))
 }
 
@@ -172,3 +180,19 @@ where
         Err(errstr(term.name))
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    use super::parse_num;
+
+    #[test]
+    fn test_parse_num() {
+        assert!(parse_num("0").is_ok());
+        assert!(parse_num("00").is_err());
+        assert!(parse_num("0000").is_err());
+        assert!(parse_num("06").is_err());
+        assert!(parse_num("+6").is_err());
+        assert!(parse_num("-6").is_err());
+    }
+}

src/miniscript/astelem.rs

@@ -429,8 +429,11 @@ where
                 Ok(expr)
             }
             ("thresh", n) => {
+                if n == 0 {
+                    return Err(errstr("no arguments given"));
+                }
                 let k = expression::terminal(&top.args[0], expression::parse_num)? as usize;
-                if n == 0 || k > n - 1 {
+                if k > n - 1 {
                     return Err(errstr("higher threshold than there are subexpressions"));
                 }
                 if n == 1 {
@@ -445,8 +448,11 @@ where
                 Ok(Terminal::Thresh(k, subs?))
             }
             ("multi", n) => {
+                if n == 0 {
+                    return Err(errstr("no arguments given"));
+                }
                 let k = expression::terminal(&top.args[0], expression::parse_num)? as usize;
-                if n == 0 || k > n - 1 {
+                if k > n - 1 {
                     return Err(errstr("higher threshold than there were keys in multi"));
                 }
 

src/policy/semantic.rs

@@ -254,6 +254,9 @@ where
                 Ok(Policy::Or(subs))
             }
             ("thresh", nsubs) => {
+                if nsubs == 0 {
+                    return Err(errstr("thresh without args"));
+                }
                 if !top.args[0].args.is_empty() {
                     return Err(errstr(top.args[0].args[0].name));
                 }