Skip to content

pcre2_compile: tighten calculation of re_blocksize #928

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.

Sign up for GitHub

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

Closed
carenas wants to merge 1 commit into from
+19 −10
Closed

pcre2_compile: tighten calculation of re_blocksize #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions src/pcre2_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10295,7 +10295,7 @@ PCRE2_SPTR ptr; /* Current pointer in pattern */
uint32_t *pptr; /* Current pointer in parsed pattern */

PCRE2_SIZE length = 1; /* Allow for final END opcode */
PCRE2_SIZE usedlength; /* Actual length used */
INT64_OR_DOUBLE usedlength; /* Actual length used */

@zherczeg zherczeg Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not use double. Just throw an error if the number is too big.

@carenas carenas Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@NWilson NWilson Aug 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

PCRE2_SIZE re_blocksize; /* Size of memory block */
PCRE2_SIZE parsed_size_needed; /* Needed for parsed pattern */

Expand Down Expand Up @@ -10853,30 +10853,39 @@ block for storing the compiled pattern and names table. Integer overflow should
no longer be possible because nowadays we limit the maximum value of
cb.names_found and cb.name_entry_size. */

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);

@zherczeg zherczeg Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about checking the error with: if (CU2BYTES(cb.names_found) > ~(PCRE2_SIZE)0 / CU2BYTES(name_entry_size))

@carenas carenas Aug 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@zherczeg zherczeg Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arguments are converted to size_t, I don't understand what is impossible.

@carenas carenas Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an integer overflow is impossible because both variables are uint16_t and more importantly they are further restricted, so names_found <= 10000 and name_entry_size <= 128


#if defined SUPPORT_WIDE_CHARS
if (cb.char_lists_size != 0)
{
#if PCRE2_CODE_UNIT_WIDTH != 32
/* Align to 32 bit first. This ensures the
allocated area will also be 32 bit aligned. */
re_blocksize = (PCRE2_SIZE)CLIST_ALIGN_TO(re_blocksize, sizeof(uint32_t));
usedlength = CLIST_ALIGN_TO(usedlength, sizeof(uint32_t));
#endif
re_blocksize += cb.char_lists_size;
usedlength += cb.char_lists_size;
}
#endif

re_blocksize += CU2BYTES(length);
if (BYTES2CU(ccontext->max_pattern_compiled_length) < length)
{
errorcode = ERR101;
cb.erroroffset = 0;
goto HAD_CB_ERROR;
}

usedlength += CU2BYTES(length);

if (re_blocksize > ccontext->max_pattern_compiled_length)
if (sizeof(size_t) == 4 &&
(usedlength > (INT64_OR_DOUBLE)ccontext->max_pattern_compiled_length ||
usedlength > (INT64_OR_DOUBLE)(SIZE_MAX - sizeof(pcre2_real_code))))
{
errorcode = ERR101;
cb.erroroffset = 0;
goto HAD_CB_ERROR;
}

re_blocksize = usedlength;
re_blocksize += sizeof(pcre2_real_code);
re = (pcre2_real_code *)
ccontext->memctl.malloc(re_blocksize, ccontext->memctl.memory_data);
Expand Down Expand Up @@ -10989,7 +10998,7 @@ memory as unaddressable, so that any out-of-bound reads can be detected. */
*code++ = OP_END;
usedlength = code - codestart;
/* LCOV_EXCL_START */
if (usedlength > length)
if (usedlength > (INT64_OR_DOUBLE)length)
{
PCRE2_DEBUG_UNREACHABLE();
errorcode = ERR23; /* Overflow of code block - internal error */
Expand Down Expand Up @@ -11071,7 +11080,7 @@ at this stage. */

#ifdef DEBUG_CALL_PRINTINT
pcre2_printint(re, stderr, TRUE);
fprintf(stderr, "Length=%lu Used=%lu\n", length, usedlength);
fprintf(stderr, "Length=%lu Used=%" PRId64 "\n", length, usedlength);
#endif

/* Unless disabled, check whether any single character iterators can be
Expand Down
2 changes: 1 addition & 1 deletion src/pcre2_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ them will be able to (i.e. assume a 64-bit world). */

/* Macro for aligning data. */
#define CLIST_ALIGN_TO(base, align) \
((base + ((size_t)(align) - 1)) & ~((size_t)(align) - 1))
((((size_t)base + align - 1) / align) * align)

/* Structure for holding information about an OP_ECLASS internal operand.
An "operand" here could be just a single OP_[X]CLASS, or it could be some
Expand Down
Loading