-
Notifications
You must be signed in to change notification settings - Fork 106
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] Out-of-bounds writes in BitView::_Read64<true>
potentially causing crashes or infinite loops
#354
Comments
const size_t totalBytes = CDiv( sizeBits, 8 );
const int32 remainderBytes = (int32)( totalBytes - fieldIndex * 8 ); I have not gone through this code in a while, but this seems to be correct. It would never exceed 8 bytes if the correct |
Consider evaluating You start with
so you get
I encountered this bug when I tried to fetch proof from a plot created with some old plotter, which is valid used with chiapos, but causes bladebit to crash. |
OK, so it seems the bug would be in |
OK. I'd like to do the modification. BTW, would you like it if I also simplify these lines this time? Lines 150 to 153 in c669876
from if( isPtrAligned && !isLastField )
field = *((uint64*)pField);
else if( !isLastField )
memcpy( &field, pField, sizeof( uint64 ) ); to if( !isLastField )
field = *((uint64*)pField); This original optimization seems unnecessary. A 64-bit dereference is a single instruction under most architectures (unless not 64-bit). Though it'll be a slower instruction if the address is unaligned, there's no reason a AFAIK, a |
Yes, please go ahead. At the time that was written I had incorrectly assumed it was invalid to read/write unaligned in arm64 as it was in the 32bit instruction set. |
@harold-b if this PR happened we can close this issue |
In the code (In the
BitView::_Read64
method withCheckAlignment==true
)bladebit/src/util/BitView.h
Lines 156 to 163 in c669876
bladebit/src/util/BitView.h
Lines 186 to 192 in c669876
The calculation of
remainderBytes
is wrong. Which causes the value ofremainderBytes
to exceeds8
potentially.So
fieldBytes[i]
may reference to some dangers zone, resulting in undefined behavior.FYI:
An example I encountered is that
remainderBytes==12
, andfieldBytes[8]
happens to references toi
, causing infinite loop. (built on Windows, debug mode)The text was updated successfully, but these errors were encountered: