Notable changes
===============
This hotfix remediates memory exhaustion vulnerabilities that zcashd inherited
as a fork of bitcoind. These bugs could allow an attacker to use peer-to-peer
messages to fill the memory of a node, resulting in a crash.
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEEX8Nd8pnYcf0pobEL9FXpuSAjoYsFAmP1CD8ACgkQ9FXpuSAj
oYvGXQwAqaa1l5qC3VY/60jkH4xmWoSUEzeCOPUG7lYJrIEyzgXj8Ko0Cjr308jm
ISXDDTOxKb2hfnCbeqbZqRyFbzGzG5L6AkjHAvmQMiZwx2JlbH2k+jd5fggOZSMv
shL5KbxWN2YRftc8r+fDraJMbGULbKBWQooKaFyQmupT+bVsRf1Nh+lFIVG4FUwA
oWZot36wB6Y99Y57wlyN2m22+j1glyk5mKv2ttXYbdwSRTFB5W5L1US6Z8uxXQPb
Qa7sVO90QtzkHr+GPtMiTn513VuLFr+KArGn+qidU/PvblJI/vXuBf54g7JZw4Ot
gkKziatgaN3pO30I4rTij78LCgKJZ/WImLE7nwTl/bG2Ki1WfyolNLjS+1pXpcPL
xlXso5ioKlSIGhnlPouXwoxlaqTpDwRKLp0azNJl5hG/tXEHupToK2M61woi9LlP
4RB+a75OptFi9NMp/Sx0T8zNRn7OB8iP+3BgRP0+mzcLC1AHfjJk/IRo9CQh5jRZ
MMe98OrC
=6QYn
-----END PGP SIGNATURE-----
Merge tag 'v5.3.3' into hotfix-v5.4.2
zcashd release v5.3.3
Notable changes
===============
This hotfix remediates memory exhaustion vulnerabilities that zcashd inherited
as a fork of bitcoind. These bugs could allow an attacker to use peer-to-peer
messages to fill the memory of a node, resulting in a crash.
little memory when we disconnect the node.
Co-authored-by: Kris Nuttycombe <kris@nutty.land>
Co-authored-by: Jack Grigg <str4d@z.cash>
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
While limitations on the influence of attackers on addrman already
exist (affected buckets are restricted to a subset based on incoming
IP / network group), there is no reason to permit them to let them
feed us addresses at more than a multiple of the normal network
rate.
This commit introduces a "token bucket" rate limiter for the
processing of addresses in incoming ADDR and ADDRV2 messages.
Every connection gets an associated token bucket. Processing an
address in an ADDR or ADDRV2 message from non-whitelisted peers
consumes a token from the bucket. If the bucket is empty, the
address is ignored (it is not forwarded or processed). The token
counter increases at a rate of 0.1 tokens per second, and will
accrue up to a maximum of 1000 tokens (the maximum we accept in a
single ADDR or ADDRV2). When a GETADDR is sent to a peer, it
immediately gets 1000 additional tokens, as we actively desire many
addresses from such peers (this may temporarily cause the token
count to exceed 1000).
The rate limit of 0.1 addr/s was chosen based on observation of
honest nodes on the network. Activity in general from most nodes
is either 0, or up to a maximum around 0.025 addr/s for recent
Bitcoin Core nodes. A few (self-identified, through subver) crawler
nodes occasionally exceed 0.1 addr/s.
(cherry-picked from commit bitcoin/bitcoin@0d64b8f709)
cs_vSend is used for two purposes - to lock the datastructures used
to queue messages to place on the wire and to only call
SendMessages once at a time per-node. I believe SendMessages used
to access some of the vSendMsg stuff, but it doesn't anymore, so
these locks do not need to be on the same mutex, and also make
deadlocking much more likely.
(cherry picked from commit bitcoin/bitcoin@d7c58ad514)
PushInventory() is currently called with a CInv object, which can be a
MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine
whether to add the hash to setInventoryTxToSend or
vInventoryBlockToSend.
Since the caller always knows what type of inventory they're pushing,
the CInv is wastefully constructed and thrown away, and tx/block relay
is being split out, we split the function into PushTxInventory() and
PushBlockInventory().
(cherry picked from commit bitcoin/bitcoin@f52d403b81)
Zcash: Adjusted to account for bitcoin/bitcoin#15759 not having been
backported.
This reduces the rate of not founds by better matching the far
end expectations, it also improves privacy by removing the
ability to use getdata to probe for a node having a txn before
it has been relayed.
(cherry picked from commit bitcoin/bitcoin@4d8993b346)
The ability to GETDATA a transaction which has not (yet) been relayed
is a privacy loss vector.
The use of the mempool for this was added as part of the mempool p2p
message and is only needed to fetch transactions returned by it.
(cherry picked from commit bitcoin/bitcoin@7e908c7b82)
This will avoid sending more pointless INVs around updates, and
prevents using filter updates to timetag transactions.
Also adds locking for fRelayTxes.
(cherry picked from commit b5599147533103efea896a1fc4ff51f2d3ad5808)
By eliminating queued entries from the mempool response and responding only at
trickle time, this makes the mempool no longer leak transaction arrival order
information (as the mempool itself is also sorted)-- at least no more than
relay itself leaks it.
(cherry picked from commit ed7068302c7490e8061cb3a558a0f83a465beeea)
Previously we used the CInv that would be sent to the peer announcing the
transaction as the key, but using the txid instead allows us to decouple the
p2p layer from the application logic (which relies on this map to avoid
duplicate tx requests).
(cherry picked from commit 7e91f632c70ff1848a152f24ee67a06796803943)
We used to have a trickle node, a node which was chosen in each iteration of
the send loop that was privileged and allowed to send out queued up non-time
critical messages. Since the removal of the fixed sleeps in the network code,
this resulted in fast and attackable treatment of such broadcasts.
This pull request changes the 3 remaining trickle use cases by random delays:
* Local address broadcast (while also removing the the wiping of the seen filter)
* Address relay
* Inv relay (for transactions; blocks are always relayed immediately)
The code is based on older commits by Patrick Strateman.
(cherry picked from commit 5400ef6bcb9d243b2b21697775aa6491115420f3)
These are (afaik) all long-standing races or concurrent accesses. Going
forward, we can clean these up so that they're not all individual atomic
accesses.
- Reintroduce cs_vRecv to guard receive-specific vars
- Lock vRecv/vSend for CNodeStats
- Make some vars atomic.
- Only set the connection time in CNode's constructor so that it doesn't change
zcash: cherry picked from commit 321d0fc6b6624c65508f8b9059418cb936f0bbbe
zcash: https://github.com/bitcoin/bitcoin/pull/9708
- Sprout and Sapling pool metrics (commitments, values)
- Block verification time histogram (as a summary, not bucketed)
- Mempool stats (same as getmempoolinfo returns)
- Inbound and outbound bytes labelled by command
- Added command labels to message counters
Replace setInventoryKnown with a rolling bloom filter
Cherry-picked from bitcoin/bitcoin#7133.
- Excluding for last commit, which needs bitcoin/bitcoin#7129.
Part of #2074.
Mruset setInventoryKnown was reduced to a remarkably small 1000
entries as a side effect of sendbuffer size reductions in 2012.
This removes setInventoryKnown filtering from merkleBlock responses
because false positives there are especially unattractive and
also because I'm not sure if there aren't race conditions around
the relay pool that would cause some transactions there to
be suppressed. (Also, ProcessGetData was accessing
setInventoryKnown without taking the required lock.)
Zcash: Was "Added -whiteconnections=<n> option" from bitcoin/bitcoin#5288. The
option was later removed in bitcoin/bitcoin#6374 which we merged in #1258. This
commit contains the difference between the two.
Kris Nuttycombe
Daira Hopwood
Pieter Wuille
Jim Posen
Matt Corallo
Dimitris Apostolou
Jack Grigg
John Newbery
Gregory Maxwell
Pieter Wuille
Suhas Daftuar
Cory Fields
Homu
Patick Strateman
Peter Todd
Josh Lehan
MarcoFalke