Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevector.h memory corruption when dealing with very large amounts of data (+/- 4GB) #11115

Closed
guidovranken opened this issue Aug 23, 2017 · 2 comments

Comments

@guidovranken
Copy link
Contributor

guidovranken commented Aug 23, 2017

Describe the issue

Can you reliably reproduce the issue?

If so, please list the steps to reproduce below:

In prevector.h

345         if (capacity() < new_size) {
346             change_capacity(new_size + (new_size >> 1));
347         }

This construct makes it prone to uint32_t (the default size prevector
uses) overflow if new_size is very high. A loop where many many
elements are added to a prevector will trigger this. std::vector does
not appear to exhibit problems when dealing with excessively large
amounts of data.

Expected behaviour

Nothing (or throwing an exception?)

Actual behaviour

Memory corruption

Screenshots.

What version of bitcoin-core are you using?

bitcoin-0.14.2 self-compiled

Machine specs:

  • OS: Linux
  • CPU:
  • RAM:
  • Disk size:
  • Disk Type (HD/SDD):

Any extra information that might be useful in the debugging process.

My prevector fuzzer ( https://github.com/guidovranken/bitcoin/blob/fuzzing/fuzzers/fuzzer-prevector.cpp ) can be used to find this. You can compile it with the flag -DSIZETYPE_UINT16_T so that prevector uses uint16_t rather than the default uint32_t as its internal size type. So rather than having to operate on buffers that are gigabytes large in order to trigger any bugs, it can be done with buffers that involve only +/- 64K buffers.

@sipa
Copy link
Member

sipa commented Aug 23, 2017

prevector has a type parameter for choosing the size type (which defaults to uint32_t). If a user of prevector uses it in a way that causes an overflow in that type, that's invalid usage of prevector.

Perhaps an overflow check can be added, though.

@maflcko
Copy link
Member

maflcko commented Apr 26, 2020

This seems like an issue of theoretical nature. I don't think we use prevectors of such size?

So closing for now, but pull requests with improvements are always welcome.

@maflcko maflcko closed this as completed Apr 26, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants