zebra icon indicating copy to clipboard operation
zebra copied to clipboard

Stop assuming testnet when parsing keys and addresses

Open teor2345 opened this issue 2 years ago • 0 comments

Scheduling

We don't need to fix these bugs until we start full wallet work. (These bugs only affect Sprout, Sapling, and Orchard keys and addresses.)

Motivation

When Zebra finds an unrecognised network in an address, it sometimes:

  • panics, or
  • assumes testnet.

This can lead to denial of service or loss of funds.

Specifications

The Zcash consensus rules say that unrecognised networks should result in an error.

Designs

In methods that handle untrusted data:

  • From
  • FromStr
  • ZcashDeserialize

For addresses and keys in:

  • Sprout
  • Sapling
  • Orchard

Stop panicking when an unrecognised human-readable part is found:

  • return an error to the caller instead
  • check all callers to make sure they don't panic

Stop assuming Mainnet or Testnet:

  • Remove Default for Network, and replace all uses with traits like ToAddressWithNetwork
  • Remove default matches (_ =>) and return an error instead
  • Check for direct uses of Mainnet or Testnet as defaults, and remove them

teor2345 avatar Jun 27 '22 02:06 teor2345