From 9594e440a5f7a6a4a8448258677fab75f564d1ab Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Tue, 1 Oct 2024 14:58:16 +0200 Subject: [PATCH 1/4] upgrade deprecated gitlab actions --- .github/workflows/fuzz.yml | 4 ++-- fuzz/generate-files.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 470bf1ac..39a1cf3c 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -51,7 +51,7 @@ roundtrip_confidential, echo "Using RUSTFLAGS $RUSTFLAGS" cd fuzz && ./fuzz.sh "${{ matrix.fuzz_target }}" - run: echo "${{ matrix.fuzz_target }}" >executed_${{ matrix.fuzz_target }} - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v4 with: name: executed_${{ matrix.fuzz_target }} path: executed_${{ matrix.fuzz_target }} @@ -62,7 +62,7 @@ roundtrip_confidential, runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: actions/download-artifact@v2 + - uses: actions/download-artifact@v4 - name: Display structure of downloaded files run: ls -R - run: find executed_* -type f -exec cat {} + | sort > executed diff --git a/fuzz/generate-files.sh b/fuzz/generate-files.sh index 4787693a..829d878c 100755 --- a/fuzz/generate-files.sh +++ b/fuzz/generate-files.sh @@ -82,7 +82,7 @@ $(for name in $(listTargetNames); do echo "$name,"; done) echo "Using RUSTFLAGS \$RUSTFLAGS" cd fuzz && ./fuzz.sh "\${{ matrix.fuzz_target }}" - run: echo "\${{ matrix.fuzz_target }}" >executed_\${{ matrix.fuzz_target }} - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v4 with: name: executed_\${{ matrix.fuzz_target }} path: executed_\${{ matrix.fuzz_target }} From e4e7ffebd5b29fc50e645dde53dd9a5b216d958c Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 2 Oct 2024 11:47:44 +0200 Subject: [PATCH 2/4] filter out also non-printable ascii chars --- src/descriptor/tr.rs | 7 ++----- src/expression.rs | 10 +++------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 2b7b5c2f..0ff19190 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -4,6 +4,7 @@ use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::{fmt, hash}; +use bitcoin_miniscript::expression::check_valid_chars; use elements::taproot::{ LeafVersion, TaprootBuilder, TaprootSpendInfo, TAPROOT_CONTROL_BASE_SIZE, TAPROOT_CONTROL_MAX_NODE_COUNT, TAPROOT_CONTROL_NODE_SIZE, @@ -716,11 +717,7 @@ impl fmt::Display for Tr { // Helper function to parse string into miniscript tree form fn parse_tr_tree(s: &str) -> Result, Error> { - for ch in s.bytes() { - if !ch.is_ascii() { - return Err(Error::Unprintable(ch)); - } - } + check_valid_chars(s)?; if s.len() > 5 && &s[..5] == "eltr(" && s.as_bytes()[s.len() - 1] == b')' { let rest = &s[5..s.len() - 1]; diff --git a/src/expression.rs b/src/expression.rs index da8c1f18..a884c78f 100644 --- a/src/expression.rs +++ b/src/expression.rs @@ -7,6 +7,8 @@ use std::fmt; use std::str::FromStr; +use bitcoin_miniscript::expression::check_valid_chars; + use crate::{errstr, Error, MAX_RECURSION_DEPTH}; #[derive(Debug, Clone)] @@ -202,13 +204,7 @@ impl<'a> Tree<'a> { /// Parses a tree from a string #[allow(clippy::should_implement_trait)] // seems to be a false positive pub fn from_str(s: &'a str) -> Result, Error> { - // Filter out non-ASCII because we byte-index strings all over the - // place and Rust gets very upsbt 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() { From ea86cb50664da290d5602d6abaa3ef5c289e395c Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 2 Oct 2024 12:01:58 +0200 Subject: [PATCH 3/4] replace other Unprintable check with upstream impl --- src/descriptor/checksum.rs | 8 +++----- src/policy/concrete.rs | 7 ++----- src/policy/semantic.rs | 7 ++----- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 0cef0390..85ee4ec4 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -8,6 +8,8 @@ use core::fmt; use core::iter::FromIterator; +use bitcoin_miniscript::expression::check_valid_chars; + use crate::Error; const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ "; @@ -51,11 +53,7 @@ pub fn desc_checksum(desc: &str) -> Result { /// if it is present and returns the descriptor string /// without the checksum pub(crate) fn verify_checksum(s: &str) -> Result<&str, Error> { - for ch in s.as_bytes() { - if *ch < 20 || *ch > 127 { - return Err(Error::Unprintable(*ch)); - } - } + check_valid_chars(s)?; let mut parts = s.splitn(2, '#'); let desc_str = parts.next().unwrap(); diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index ab89af78..85f90b01 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -7,6 +7,7 @@ use std::collections::HashSet; use std::{error, fmt, str}; +use bitcoin_miniscript::expression::check_valid_chars; use elements::{LockTime, Sequence}; #[cfg(feature = "compiler")] use { @@ -1098,11 +1099,7 @@ impl_from_str!( Policy, type Err = Error;, fn from_str(s: &str) -> Result, Error> { - for ch in s.as_bytes() { - if *ch < 20 || *ch > 127 { - return Err(Error::Unprintable(*ch)); - } - } + check_valid_chars(s)?; let tree = expression::Tree::from_str(s)?; let policy: Policy = FromTree::from_tree(&tree)?; diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 3e7e5325..16dc1208 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -6,6 +6,7 @@ use std::str::FromStr; use std::{fmt, str}; +use bitcoin_miniscript::expression::check_valid_chars; use elements::{LockTime, Sequence}; use super::concrete::PolicyError; @@ -287,11 +288,7 @@ impl_from_str!( Policy, type Err = Error;, fn from_str(s: &str) -> Result, Error> { - for ch in s.as_bytes() { - if *ch < 20 || *ch > 127 { - return Err(Error::Unprintable(*ch)); - } - } + check_valid_chars(s)?; let tree = expression::Tree::from_str(s)?; expression::FromTree::from_tree(&tree) From 03650dd4fbdd659202a17145bf06aa8fc27cc162 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 2 Oct 2024 12:06:11 +0200 Subject: [PATCH 4/4] Remove unused error variant --- src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3e1a49e6..e931c09c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -296,8 +296,6 @@ pub enum Error { CmsTooManyKeys(u32), /// A tapscript multi_a cannot support more than MAX_BLOCK_WEIGHT/32 keys MultiATooManyKeys(u32), - /// Encountered unprintable character in descriptor - Unprintable(u8), /// expected character while parsing descriptor; didn't find one ExpectedChar(char), /// While parsing backward, hit beginning of script @@ -463,7 +461,6 @@ impl fmt::Display for Error { Error::Script(ref e) => fmt::Display::fmt(e, f), Error::AddrError(ref e) => fmt::Display::fmt(e, f), Error::CmsTooManyKeys(n) => write!(f, "checkmultisig with {} keys", n), - Error::Unprintable(x) => write!(f, "unprintable character 0x{:02x}", x), Error::ExpectedChar(c) => write!(f, "expected {}", c), Error::UnexpectedStart => f.write_str("unexpected start of script"), Error::Unexpected(ref s) => write!(f, "unexpected «{}»", s), @@ -537,7 +534,6 @@ impl error::Error for Error { | InvalidPush(_) | CmsTooManyKeys(_) | MultiATooManyKeys(_) - | Unprintable(_) | ExpectedChar(_) | UnexpectedStart | Unexpected(_)