librustzcash
librustzcash copied to clipboard
Document potential memory use of functions that deserialize CompactSize-containing data structures
(Originally reported by @teor2345 through security channels.)
The maximum value of a CompactSize field is 32MiB. Parsers that make use of the CompactSize value to pre-allocate memory for parsing the remainder of the data structure are at risk of creating a memory denial of service attack, especially when those parsers are parallelized and run in direct response to network traffic (such as might be the case in zebrad).
Wherever this library does preallocation like this, the maximum potential memory use should be documented so that developers who call the function are aware of the potential for memory DoS.
There's another issue we noticed during the Arborist call today: CompactSize
as inherited from Bitcoin Core has a limit of 2^26, but it is used in FlyClient nodes to (among other things) count the number of Sapling transactions. This could be up to 2^32 (if we had one Sapling output per transaction), and in any case could definitely exceed the current CompactSize
limit.
In practice this shouldn't be an issue (as FlyClient history trees reset roughly every 6 months at each NU), but it's a spec issue because the types allow it to occur. More concerningly, the CompactSize limit has existed in upstream Bitcoin Core since the beginning (at least since the migration to a git repo), but the Bitcoin developer docs don't include that limit.
We need to decide how to handle these varied usages of CompactSize, what the limits are (or should be) in its various (mostly consensus-critical) usages, and then specify it ourselves.