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

[Bug] Remaining deserialization issues #2097

Open
3 of 4 tasks
ljedrz opened this issue Oct 16, 2023 · 1 comment
Open
3 of 4 tasks

[Bug] Remaining deserialization issues #2097

ljedrz opened this issue Oct 16, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@ljedrz
Copy link
Contributor

ljedrz commented Oct 16, 2023

Other than the pending changes in https://github.com/AleoHQ/snarkVM/pull/1985 (including the ones in the review comments), the following issues were found via snarkOS network message fuzzing:

  • the size member of EvaluationDomain is used to resize vectors; it's u64, which can be maliciously crafted to be large enough to cause an OOM
  • proposed solution: manually implement CanonicalDeserialize and limit the maximum allowed size to some concrete value or a smaller data type
  • The degree of the EpochChallenge, which is derived from the epoch_polynomial member, can cause an OOM when used in hash_to_polynomial (and ultimately in hash_to_coefficients), as it's used to determine the number of coefficients collected into a vector
  • proposed solution: check for a fixed maximum value for the degree after it's read in EpochChallenge::read_le or reduce its data type below u32
  • (varuna) Commitments::deserialize_with_mode can panic due to integer overflow when calling batch_sizes.iter().sum() at the very beginning
  • proposed solution: change that line to let mut w = Vec::new();
  • BatchCertificate::read_le needs to use IndexMap::new instead of ::with_capacity (no longer an issue with the 2nd version of this object, and the 1st one is going to be removed)
@ljedrz ljedrz added the bug Something isn't working label Oct 16, 2023
@ljedrz
Copy link
Contributor Author

ljedrz commented Nov 13, 2023

I'm now fuzzing the current version; the 1st point is no longer applicable, the next 2 can still cause an OOM, and the recent deserialization patches omitted a bit in the BatchCertificate, probably because it's being deprecated (added to the list).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant