pcre2_compile: tighten calculation of re_blocksize - #928
Conversation
1b5a9ac to
8d0e4d8
Compare
|
|
||
| PCRE2_SIZE length = 1; /* Allow for final END opcode */ | ||
| PCRE2_SIZE usedlength; /* Actual length used */ | ||
| INT64_OR_DOUBLE usedlength; /* Actual length used */ |
There was a problem hiding this comment.
I would not use double. Just throw an error if the number is too big.
There was a problem hiding this comment.
For all purposes (specially since the codebase is C99 and most CPUs are 64bit) this is equivalent to int64_t which is of course what we need to check for integer overflows, specially when size_t is 32bit
There was a problem hiding this comment.
I'm actually planning to get rid of INT64_OR_DOUBLE. It's only really used in one place, and it isn't actually required there, so ideally it would just disappear.
to avoid integer overflows (specially in 32-bit), use a 64-bit integer (or equivalent) to add all sections.
|
|
||
| re_blocksize = | ||
| CU2BYTES((PCRE2_SIZE)cb.names_found * (PCRE2_SIZE)cb.name_entry_size); | ||
| usedlength = CU2BYTES((size_t)cb.names_found * cb.name_entry_size); |
There was a problem hiding this comment.
What about checking the error with: if (CU2BYTES(cb.names_found) > ~(PCRE2_SIZE)0 / CU2BYTES(name_entry_size))
There was a problem hiding this comment.
that would be imposible, since both of those are uint16_t, and AFAIK name_entry_size can't be over 128, but agree that the way the checking is done is convoluted and seem to leave gaps.
There was a problem hiding this comment.
The arguments are converted to size_t, I don't understand what is impossible.
|
@carenas, would you mind if I had a go at this issue too? I'm digging deeper, and want to clean up and fix quite a few unchecked additions in the same area. I don't want to ignore your work here, but I would prefer to make a less targeted (broader) fix. |
3 participants
Fixes: #918