Soundness issue: Macros should overflow-check integer parameters #26
Comments
|
I believe I've fixed this, and even made this sum guaranteed to be computed at compile time by changing |
|
Since in Rust no slice can have more than isize::MAX element unless its elements are ZSTs, and the code doesn't do UB with ZSTs, I would say it is fine -- however, I think it would be better to just panic in the overflow case, instead of returning the maximum usize. Relatedly -- why not |
|
The issue is that I'm not sure how to panic in a const function. I suppose
the answer is to put a not const assert that the min len is less than the
max.
David Roundy
…On Sat, Jul 20, 2024, 2:07 AM Luca Versari ***@***.***> wrote:
Since in Rust no slice can have more than isize::MAX element unless its
elements are ZSTs, and the code doesn't do UB with ZSTs, I would say it is
fine -- however, I think it would be better to just panic in the overflow
case, instead of returning the maximum usize.
Relatedly -- why not saturating_add if you were going for saturating
semantics?
—
Reply to this email directly, view it on GitHub
<#26 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABBSKNS43CTIVRNYSZFDADZNISERAVCNFSM6AAAAABINVNRAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRGA2TGNBVGY>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
You can just |
|
I switched to saturating add and added an explicit panic afterwards, which seems a reasonable solution. Somehow after seeing that |
|
This should work as far as I can tell! |
|
Thanks!
David Roundy
…On Sat, Jul 20, 2024, 7:08 AM Luca Versari ***@***.***> wrote:
This should work as far as I can tell!
—
Reply to this email directly, view it on GitHub
<#26 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABBSKMYBMCAMWXCTDI2NYLZNJVOZAVCNFSM6AAAAABINVNRAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRGE3DEOJTGA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Found by @veluca93
In the two
_refsmacros,$preand$postare added for a bounds check, however there is nothing that checks if they overflow.arrayref/src/lib.rs
Lines 112 to 113 in 6a0d584
It's pretty hard to actually trigger this since very-large arrays are not quite allowed by the compiler anyway on at least x86, but it is worth guarding against.
The text was updated successfully, but these errors were encountered: