-
Notifications
You must be signed in to change notification settings - Fork 59
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
Fixed MemoryError when decoding large definite strings #204
Conversation
I fuzzed this branch a bit, and it produced the following crash: Output
I've attached the crash here: crash-54a4c03551e10f514542bdf35c00142e617b1ff1.txt |
Would you be interested in having your project be a part of OSS-Fuzz? That could help with automatically finding crashes and bugs for you. I could take care of adding it as a new project - all I'd need is a primary contact email address. |
Sounds okay. Is it comparable with Hypothesis? |
And my primary contact address is alex.gronholm@nextday.fi. |
Hmm, the adding new projects docs say that a Google account is required. Is that email address connected to a Google account by chance? Fuzzing testing is similar in some ways to Hypothesis. OSS-Fuzz is a project by Google to provide free compute cycles to fuzz OSS software. For more information on fuzzing, I'd recommend starting here. |
Yes, that's associated with a Google account. |
What do I do with this if I want to reproduce the crash? |
I tested it out like this:
|
Alright, I was able to reproduce the crash here. I think I need to start fuzzing the code before I push anything to master. |
Looks like this is just another case of allocating a huge amount of memory (4629771061636907009) bytes, but this triggers a |
Turns out that the problem wasn't triggered with a |
All other Python CBOR implementations I tested with also raised a |
I have a fix that avoids |
Alright, what's left now is to fuzz this branch before I merge. |
I'm not getting any |
I found another crash with the following file: crash-c528afcec87be909de91322a14693702fd1d44a0.txt I think I'm correctly fuzzing this branch, but I'm not sure. Are you able to reproduce the crash? |
Yeah, I can reproduce it. Looking into it now. |
No, wait, I forgot to recompile after switching branches. I'm getting this instead now: |
I'm not seeing anything bad in this branch after the commit I just pushed a little while ago, so I'm merging it. |
`cbor2` is a Python CBOR (de)serializer with extensive tag support. It has nearly [2,000 dependents](https://github.com/agronholm/cbor2/network/dependents) on GitHub. It's used by many large blockchain projects, such as: - https://github.com/bitcoin-core/HWI - https://github.com/Blockstream/Jade - https://github.com/vyperlang/vyper - https://github.com/lidofinance/lido-dao - https://github.com/fetchai/cosmpy - https://github.com/OpShin/opshin And other non-blockchain projects, such as: - https://github.com/project-chip/connectedhomeip - https://github.com/pritunl/pritunl - https://github.com/amazon-ion/ion-python I received approval from this project here: agronholm/cbor2#204 (comment)
Looks like the OSS-Fuzz PR went through: google/oss-fuzz#11444 🎉 |
Relates to #198.