-
Notifications
You must be signed in to change notification settings - Fork 216
fix(store): prevent delete out of bounds in spliceDynamicData
#3521
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
Conversation
|
3b20599 to
10bbab5
Compare
10bbab5 to
3ff178f
Compare
3ff178f to
5027f3e
Compare
|
combining all changes that require an audit into #3630 |
In
spliceDynamicData, we’re checking thatstartis within the bounds of the previous field length but aren’t consideringdeleteCountin that check. There is another check that checks thatstart + deleteCountlines up with the previous length of the field if the total length of the field changed, but this only applies if the length changed. That means if the length of the data to insert is the same asdeleteCount, it is possible to “insert data after the length of the field” (ie by settingstartto the end of the field). I put “insert data after the length of the field” in quotes, since the length of the field is not actually changed, which means when retrieving the whole field onchain the data appended at the end would not be included, similar to how items that are pop’ed from a dynamic field are not actually cleared from storage but just the field length is reduced.But means indexers/clients need to be aware of this nuance and use
encodedLengthsas source of truth (like we do onchain).We can remove this edge case by changing the check to
if(startWithinField > previousFieldLength - deleteCount).When using our table libraries this does not happen since they don't call
spliceDynamicDatawith an invalidstartvalue, but it’s possible to trigger this by callingworld.spliceDynamicDatamanually.