Merge pull request #5484 from str4d/5466-ua-validation
Fully-validate contents of UAs and UFVKs on parsing
This commit is contained in:
commit
6b335ecb4c
|
@ -269,6 +269,12 @@ version = "1.1.0"
|
||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "c4872d67bab6358e59559027aa3b9157c53d9358c51423c17554809a8858e0f8"
|
checksum = "c4872d67bab6358e59559027aa3b9157c53d9358c51423c17554809a8858e0f8"
|
||||||
|
|
||||||
|
[[package]]
|
||||||
|
name = "cc"
|
||||||
|
version = "1.0.72"
|
||||||
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
|
checksum = "22a9137b95ea06864e018375b72adfb7db6e6f68cfc8df5a04d00288050485ee"
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "cfg-if"
|
name = "cfg-if"
|
||||||
version = "1.0.0"
|
version = "1.0.0"
|
||||||
|
@ -803,6 +809,7 @@ dependencies = [
|
||||||
"nonempty",
|
"nonempty",
|
||||||
"orchard",
|
"orchard",
|
||||||
"rand_core 0.6.3",
|
"rand_core 0.6.3",
|
||||||
|
"secp256k1",
|
||||||
"subtle",
|
"subtle",
|
||||||
"thiserror",
|
"thiserror",
|
||||||
"tokio",
|
"tokio",
|
||||||
|
@ -1403,6 +1410,24 @@ version = "1.1.0"
|
||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
|
checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
|
||||||
|
|
||||||
|
[[package]]
|
||||||
|
name = "secp256k1"
|
||||||
|
version = "0.20.3"
|
||||||
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
|
checksum = "97d03ceae636d0fed5bae6a7f4f664354c5f4fcedf6eef053fef17e49f837d0a"
|
||||||
|
dependencies = [
|
||||||
|
"secp256k1-sys",
|
||||||
|
]
|
||||||
|
|
||||||
|
[[package]]
|
||||||
|
name = "secp256k1-sys"
|
||||||
|
version = "0.4.2"
|
||||||
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
|
checksum = "957da2573cde917463ece3570eab4a0b3f19de6f1646cde62e6fd3868f566036"
|
||||||
|
dependencies = [
|
||||||
|
"cc",
|
||||||
|
]
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "serde"
|
name = "serde"
|
||||||
version = "1.0.131"
|
version = "1.0.131"
|
||||||
|
|
|
@ -38,6 +38,7 @@ jubjub = "0.8"
|
||||||
memuse = "0.2"
|
memuse = "0.2"
|
||||||
nonempty = "0.7"
|
nonempty = "0.7"
|
||||||
orchard = "=0.1.0-beta.1"
|
orchard = "=0.1.0-beta.1"
|
||||||
|
secp256k1 = "0.20"
|
||||||
subtle = "2.2"
|
subtle = "2.2"
|
||||||
rand_core = "0.6"
|
rand_core = "0.6"
|
||||||
tracing = "0.1"
|
tracing = "0.1"
|
||||||
|
|
|
@ -39,7 +39,14 @@ if ENABLE_WALLET
|
||||||
LIBBITCOIN_WALLET=libbitcoin_wallet.a
|
LIBBITCOIN_WALLET=libbitcoin_wallet.a
|
||||||
endif
|
endif
|
||||||
|
|
||||||
RUST_ENV_VARS = RUSTC="$(RUSTC)" TERM=dumb
|
# We depend on the secp256k1 crate for some logic on the Rust side of the FFI. This crate
|
||||||
|
# is a wrapper around libsecp256k1, which we already vendor in our code; the crate vendors
|
||||||
|
# its own copy with non-colliding symbols. To ensure that we only use a single version of
|
||||||
|
# libsecp256k1, we disable symbol renaming in the secp256k1-sys crate so it links to the
|
||||||
|
# same library as the C++ code.
|
||||||
|
# - Note that this does not prevent the secp256k1-sys vendored code from being built; this
|
||||||
|
# requires https://github.com/rust-bitcoin/rust-secp256k1/issues/380 to be addressed.
|
||||||
|
RUST_ENV_VARS = RUSTC="$(RUSTC)" TERM=dumb RUSTFLAGS="--cfg=rust_secp_no_symbol_renaming"
|
||||||
RUST_BUILD_OPTS = --lib --release --target $(RUST_TARGET)
|
RUST_BUILD_OPTS = --lib --release --target $(RUST_TARGET)
|
||||||
|
|
||||||
rust_verbose = $(rust_verbose_@AM_V@)
|
rust_verbose = $(rust_verbose_@AM_V@)
|
||||||
|
@ -507,6 +514,7 @@ zcash_cli_LDADD = \
|
||||||
$(LIBZCASH) \
|
$(LIBZCASH) \
|
||||||
$(LIBRUSTZCASH) \
|
$(LIBRUSTZCASH) \
|
||||||
$(LIBBITCOIN_CRYPTO) \
|
$(LIBBITCOIN_CRYPTO) \
|
||||||
|
$(LIBSECP256K1) \
|
||||||
$(LIBZCASH_LIBS)
|
$(LIBZCASH_LIBS)
|
||||||
#
|
#
|
||||||
|
|
||||||
|
|
|
@ -76,15 +76,16 @@ impl UnifiedAddressHelper {
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|receiver| match receiver {
|
.map(|receiver| match receiver {
|
||||||
unified::Receiver::Orchard(data) => {
|
unified::Receiver::Orchard(data) => {
|
||||||
// ZIP 316: Senders MUST reject Unified Addresses in which any
|
// ZIP 316: Consumers MUST reject Unified Addresses/Viewing Keys in
|
||||||
// constituent address does not meet the validation requirements of
|
// which any constituent Item does not meet the validation
|
||||||
// its Receiver Encoding.
|
// requirements of its encoding.
|
||||||
// TODO: Add this API to the orchard crate.
|
if orchard::Address::from_raw_address_bytes(&data)
|
||||||
// if let Err(e) = orchard::Address::from_bytes(data) {
|
.is_none()
|
||||||
// tracing::error!("{}", e);
|
.into()
|
||||||
// false
|
|
||||||
// } else {
|
|
||||||
{
|
{
|
||||||
|
tracing::error!("Unified Address contains invalid Orchard receiver");
|
||||||
|
false
|
||||||
|
} else {
|
||||||
unsafe {
|
unsafe {
|
||||||
// TODO: Replace with Orchard support.
|
// TODO: Replace with Orchard support.
|
||||||
(unknown_cb.unwrap())(ua_obj, 0x03, data.as_ptr(), data.len())
|
(unknown_cb.unwrap())(ua_obj, 0x03, data.as_ptr(), data.len())
|
||||||
|
@ -92,9 +93,9 @@ impl UnifiedAddressHelper {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
unified::Receiver::Sapling(data) => {
|
unified::Receiver::Sapling(data) => {
|
||||||
// ZIP 316: Senders MUST reject Unified Addresses in which any
|
// ZIP 316: Consumers MUST reject Unified Addresses/Viewing Keys in
|
||||||
// constituent address does not meet the validation requirements of
|
// which any constituent Item does not meet the validation
|
||||||
// its Receiver Encoding.
|
// requirements of its encoding.
|
||||||
if sapling::PaymentAddress::from_bytes(&data).is_none() {
|
if sapling::PaymentAddress::from_bytes(&data).is_none() {
|
||||||
tracing::error!("Unified Address contains invalid Sapling receiver");
|
tracing::error!("Unified Address contains invalid Sapling receiver");
|
||||||
false
|
false
|
||||||
|
|
|
@ -34,7 +34,43 @@ pub extern "C" fn unified_full_viewing_key_parse(
|
||||||
|
|
||||||
match unsafe { CStr::from_ptr(encoded) }.to_str() {
|
match unsafe { CStr::from_ptr(encoded) }.to_str() {
|
||||||
Ok(encoded) => match Ufvk::decode(encoded) {
|
Ok(encoded) => match Ufvk::decode(encoded) {
|
||||||
Ok((parsed_network, fvk)) if parsed_network == network => Box::into_raw(Box::new(fvk)),
|
Ok((parsed_network, fvk)) if parsed_network == network => {
|
||||||
|
// ZIP 316: Consumers MUST reject Unified Addresses/Viewing Keys in which
|
||||||
|
// any constituent Item does not meet the validation requirements of its
|
||||||
|
// encoding.
|
||||||
|
for item in fvk.items() {
|
||||||
|
match item {
|
||||||
|
Fvk::Orchard(data) => {
|
||||||
|
if orchard::keys::FullViewingKey::from_bytes(&data).is_none() {
|
||||||
|
error!("Unified FVK contains invalid Orchard FVK");
|
||||||
|
return std::ptr::null_mut();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Fvk::Sapling(data) => {
|
||||||
|
// The last 32 bytes is the diversifier key, which is opaque.
|
||||||
|
// The remaining 96 bytes should be a valid Sapling FVK.
|
||||||
|
if zcash_primitives::sapling::keys::FullViewingKey::read(&data[..96])
|
||||||
|
.is_err()
|
||||||
|
{
|
||||||
|
error!("Unified FVK contains invalid Sapling FVK");
|
||||||
|
return std::ptr::null_mut();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Fvk::P2pkh(data) => {
|
||||||
|
// The first 32 bytes is the chaincode, which is opaque.
|
||||||
|
// The remaining 33 bytes should be the compressed encoding of
|
||||||
|
// a secp256k1 point.
|
||||||
|
if let Err(e) = secp256k1::PublicKey::from_slice(&data[32..]) {
|
||||||
|
error!("Unified FVK contains invalid transparent FVK: {}", e);
|
||||||
|
return std::ptr::null_mut();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Can't check anything for unknown typecodes.
|
||||||
|
Fvk::Unknown { .. } => (),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Box::into_raw(Box::new(fvk))
|
||||||
|
}
|
||||||
Ok((parsed_network, _)) => {
|
Ok((parsed_network, _)) => {
|
||||||
error!(
|
error!(
|
||||||
"Key was encoded for a different network ({:?}) than what was requested ({:?})",
|
"Key was encoded for a different network ({:?}) than what was requested ({:?})",
|
||||||
|
|
Loading…
Reference in New Issue