Skip to content

Commit

Permalink
Merge bitcoin#25540: miniscript: avoid wasteful computation, prevent …
Browse files Browse the repository at this point in the history
…memory blowup when fuzzing

e8cc2e4 Make miniscript string parsing account for exact script size as bound (Pieter Wuille)
4cb8f9a Permit delaying duplicate key check in miniscript::Node construction (Pieter Wuille)

Pull request description:

  As reported in bitcoin#24860 (comment), the current code to construct a `miniscript::Node` could cause a blowup on large fuzzer inputs. This is because:
  1. The duplicate key check is redundantly done at parsing time, since we will recursively create miniscript nodes and the constructor will unconditionally look for duplicate across this node's keys and all its sub-nodes'.
  2. We don't put an upper bound on the size of the inputs to consider for parsing.

  To avoid wasteful computation, and prevent the blowup on some fuzzer inputs, limit the size of reasonable inputs and only perform the check for duplicate keys once when parsing.
  Regarding the duplicate key check bypass in the constructor we iterated on different approaches, and eventually settled on passing a dummy argument. Albeit less elegant, all other approaches required getting rid of `std::make_shared` and adding an allocation *per node created*.

  This PR contains code from Pieter Wuille (see commits).

  Fixes bitcoin#25824.

ACKs for top commit:
  darosior:
    ACK e8cc2e4 -- it's my own PR but most of the code here was written by sipa. I've reviewed and tested it.
  sipa:
    ACK e8cc2e4 (for the few parts of the code that aren't mine)

Tree-SHA512: c21de39b3eeb484393758629882fcf8694a9bd1b8f15ae22efcec1582efc9c2309c5a0c2d90f361dd8e233d704a07dcd5fb982f4a48a002c4d8789e1d78bb526
  • Loading branch information
glozow committed Sep 19, 2022
2 parents a9ffebd + e8cc2e4 commit 55e1deb
Show file tree
Hide file tree
Showing 2 changed files with 194 additions and 91 deletions.
Loading

0 comments on commit 55e1deb

Please sign in to comment.