1489408 - Consider using CheckedInt in dom/indexedDB/ActorsParent.cpp

Status: NEW

(Core :: Storage: IndexedDB, enhancement, P4)

Description

[Tom Tung [:tt, :ttung]]

CheckedInt [1] provides comparatively more protects on preventing overflow. For instance, it supports more operators' checks. Also, we can use it to protect from being overflow directly, rather than having an additional check for the value. So that it reduces the possibility for the mismatch between the practical computation and the overflow check.

Perhaps, we might consider applying this format for all the overflow checks in dom/indexedDB/ActorsParent.cpp

If it's okay to do that, I reckon it might be a good first bug.

[1] https://searchfox.org/mozilla-central/source/mfbt/CheckedInt.h

Comment 1

[Tom Tung [:tt, :ttung]]

I tried to catch all the overflow checks in indexedDB/ActorsParent.cpp by using "_MAX" and "_MIN" to search and I got 23 cases. I listed them below:

Inside a comment -> don't need to use CheckedInt
1. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#10

Inside a static assert -> don't need to use CheckedInt
2. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#216

MOZ_ASSERT; only execute in the DEBUG build -> don't need to use CheckedInt
3. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#654
4. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#725
5. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#754
6. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#8988
7. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#9003
8. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#10596
9. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#11932
10. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#11965
11. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#11968
12. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#13940
13. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#15249
14. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#8990

infoLength -> needed, but it should be taken care of in another issue
15. https://searchfox.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp#792

blobLength -> needed
16. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#807-813

UINTPTR -> not sure if it's covered by CheckedInt
17. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#865

It's used to ensure keyBufferLength isn't consumed more than a half of its size; it's not an overflow check -> don't need to use CheckedInt
18. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#888
19. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#3864

It's used to ensure sortKeyBufferLength isn't consumed more than a half of its size; it's not an overflow check -> don't need to use CheckedInt
20. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#907

It's used for deciding the value of delay; it's not an overflow check -> don't need to use CheckedInt
21. https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/dom/indexedDB/ActorsParent.cpp#12221

mNextObjectStoreId, mNextIndexId -> might consider, but it's not really necessary.
22, 23. https://searchfox.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp#21825-21832

Overall, I reckon we can apply CheckedInt for blobLength, and maybe also mNextObjectStoreId and mNextIndexId in this bug.