-
Notifications
You must be signed in to change notification settings - Fork 17
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
inclusion, all: RoundDownPowerOfTwo and RoundUpPowerOfTwo are very inefficient and can cause running out of RAM due to an integer overflow #117
Comments
odeke-em
added a commit
to orijtech/go-square
that referenced
this issue
Oct 17, 2024
This code fixes an integer flow using finer bit twiddling and even makes it much more efficient to always run deterministically in O(1) time essentially and not O(k) where k=log2(input) due to the prior k iterations that then caused the overflow when result became > maxInt. While here also added some benchmarks and more test cases. Fixes celestiaorg#117
odeke-em
added a commit
to orijtech/go-square
that referenced
this issue
Oct 17, 2024
This code fixes an integer flow using finer bit twiddling and even makes it much more efficient to always run deterministically in O(1) time essentially and not O(k) where k=log2(input) due to the prior k iterations that then caused the overflow when result became > maxInt. While here also added some benchmarks and more test cases. Fixes celestiaorg#117
odeke-em
added a commit
to orijtech/go-square
that referenced
this issue
Oct 17, 2024
This code fixes an integer flow using finer bit twiddling and even makes it much more efficient to always run deterministically in O(1) time essentially and not O(k) where k=log2(input) due to the prior k iterations that then caused the overflow when result became > maxInt. While here also added some benchmarks and more test cases. Fixes celestiaorg#117
odeke-em
added a commit
to orijtech/go-square
that referenced
this issue
Oct 17, 2024
This code fixes an integer flow using finer bit twiddling and even makes it much more efficient to always run deterministically in O(1) time essentially and not O(k) where k=log2(input) due to the prior k iterations that then caused the overflow when result became > maxInt. While here also added some benchmarks and more test cases. Fixes celestiaorg#117
odeke-em
added a commit
to orijtech/go-square
that referenced
this issue
Oct 17, 2024
…und*PowerOfTwo This code fixes an integer flow using finer bit twiddling and even makes it much more efficient to always run deterministically in O(1) time essentially and not O(k) where k=log2(input) due to the prior k iterations that then caused the overflow when result became > maxInt. While here also added some benchmarks and more test cases. Fixes celestiaorg#117
odeke-em
added a commit
to orijtech/go-square
that referenced
this issue
Oct 17, 2024
…und*PowerOfTwo This code fixes an integer flow using finer bit twiddling and even makes it much more efficient to always run deterministically in O(1) time essentially and not O(k) where k=log2(input) due to the prior k iterations that then caused the overflow when result became > maxInt. While here also added some benchmarks and more test cases. Fixes celestiaorg#117
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary of Bug
While on a 3AM audit and vulnerability hunt, I noticed that Round*PowerOfTwo are heavily used within celestia's ecosytem by use of BlobMinSquareSize and SubTreeWidth.
If we examine the code for RoundUpPowerOfTwo, we can see this pattern
go-square/inclusion/blob_share_commitment_rules.go
Lines 52 to 59 in 8e9b275
which when read basically creates a value then per step of exponentiating 2 by 2 multiple times eventually will bump into a value less than the input, trying to get to that power of 2 in steps shall take a max of 64 loop iterations but sadly in that code there is an integer overflow when you pass in a signed integer
int*
due to this checkso when you have for example input=1<<64 - 1; that code starts with result=1 and in each step result is bit shifted to the left which is exponentiation for 2; but when we hit 1<<63; that value is still less than 1<<63 and in the next loop becomes 1<<64 and oops, maInt = 1<<64 - 1 hence that value overflows and becomes negative :-( -9223372036854775808 so that loop then continues and now the negative value when multiplied by 2 again warps around back to 0 but hey 0 << 1 == 0 and that loop iterates until you run out of RAM or your system kills it
Remedy
We can make that code so much more efficient and not suspectible to any integer overflows using knowledge from bit shifting and fiddling by noticing a key component:
which run in basically constant time and not susceptible to integer overflows and will always run in deterministic time and should actually have a positive impact on Celestia given that the callers for those functions are hot and the crux of Celestia
Kindly cc-ing @Wondertan @walldiss @liamsi @veorq
Version
8e9b275
Steps to Reproduce
Run this code
For Admin Use
The text was updated successfully, but these errors were encountered: