From 4d66adc387baa2d29f91f0cadfaf8d8e77e13bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= Date: Tue, 17 Jan 2023 06:55:53 -0800 Subject: [PATCH] avoid LIMIT_HEAP integer multiplication wrap around (#184) If a LIMIT_HEAP value once converted to bytes is larger than UINT_MAX would result in a bogus setting that could trigger a matching failure as shown by the following: PCRE2 version 10.42 2022-12-11 re> /(*LIMIT_HEAP=4194304)a/ data> a Failed: error -63: heap limit exceeded Remove the multiplication and instead keep track of the maximum heap allowed in KB as was done originally. Aditionally, add a check to avoid overflowing a PCRE2_SIZE while doubling the heap used and that could result in a crash (only on systems with a 32-bit PCRE2_SIZE and using non standard settings). Unlike the original, this code avoids rounding the heapframes_size to the frame_size at the allocation time, which simplifies the logic and wasn't really needed. Fixes: d90fb23 (Refactor match_data() to always use the heap instead of having an initial frames vector on the stack..., 2022-07-27) Closes: #183 --- src/pcre2_intmodedep.h | 4 ++-- src/pcre2_match.c | 38 +++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/pcre2_intmodedep.h b/src/pcre2_intmodedep.h index 390e737a6..8f6487e97 100644 --- a/src/pcre2_intmodedep.h +++ b/src/pcre2_intmodedep.h @@ -858,7 +858,7 @@ doing traditional NFA matching (pcre2_match() and friends). */ typedef struct match_block { pcre2_memctl memctl; /* For general use */ - PCRE2_SIZE heap_limit; /* As it says */ + uint32_t heap_limit; /* As it says */ uint32_t match_limit; /* As it says */ uint32_t match_limit_depth; /* As it says */ uint32_t match_call_count; /* Number of times a new frame is created */ @@ -911,7 +911,7 @@ typedef struct dfa_match_block { PCRE2_SPTR last_used_ptr; /* Latest consulted character */ const uint8_t *tables; /* Character tables */ PCRE2_SIZE start_offset; /* The start offset value */ - PCRE2_SIZE heap_limit; /* As it says */ + uint32_t heap_limit; /* As it says */ PCRE2_SIZE heap_used; /* As it says */ uint32_t match_limit; /* As it says */ uint32_t match_limit_depth; /* As it says */ diff --git a/src/pcre2_match.c b/src/pcre2_match.c index 168b9fad0..fa49e64a0 100644 --- a/src/pcre2_match.c +++ b/src/pcre2_match.c @@ -665,13 +665,28 @@ N = (heapframe *)((char *)F + frame_size); if (N >= frames_top) { heapframe *new; - PCRE2_SIZE newsize = match_data->heapframes_size * 2; + PCRE2_SIZE newsize; - if (newsize > mb->heap_limit) + if (match_data->heapframes_size >= PCRE2_SIZE_MAX / 2) { - PCRE2_SIZE maxsize = (mb->heap_limit/frame_size) * frame_size; - if (match_data->heapframes_size >= maxsize) return PCRE2_ERROR_HEAPLIMIT; - newsize = maxsize; + if (match_data->heapframes_size == PCRE2_SIZE_MAX - 1) + return PCRE2_ERROR_NOMEMORY; + newsize = PCRE2_SIZE_MAX - 1; + } + else + newsize = match_data->heapframes_size * 2; + + if (newsize / 1024 >= mb->heap_limit) + { + PCRE2_SIZE old_size = match_data->heapframes_size / 1024; + if (mb->heap_limit <= old_size) return PCRE2_ERROR_HEAPLIMIT; + else + { + PCRE2_SIZE max_delta = 1024 * (mb->heap_limit - old_size); + int over_bytes = match_data->heapframes_size % 1024; + if (over_bytes) max_delta -= (1024 - over_bytes); + newsize = match_data->heapframes_size + max_delta; + } } new = match_data->memctl.malloc(newsize, match_data->memctl.memory_data); @@ -6801,7 +6816,7 @@ the pattern. It is not used at all if there are no capturing parentheses. frame_size is the total size of each frame match_data->heapframes is the pointer to the frames vector - match_data->heapframes_size is the total size of the vector + match_data->heapframes_size is the allocated size of the vector We must pad the frame_size for alignment to ensure subsequent frames are as aligned as heapframe. Whilst ovector is word-aligned due to being a PCRE2_SIZE @@ -6816,7 +6831,7 @@ frame_size = (offsetof(heapframe, ovector) + smaller. */ mb->heap_limit = ((mcontext->heap_limit < re->limit_heap)? - mcontext->heap_limit : re->limit_heap) * 1024; + mcontext->heap_limit : re->limit_heap); mb->match_limit = (mcontext->match_limit < re->limit_match)? mcontext->match_limit : re->limit_match; @@ -6832,14 +6847,15 @@ the size to a multiple of the frame size. */ heapframes_size = frame_size * 10; if (heapframes_size < START_FRAMES_SIZE) heapframes_size = START_FRAMES_SIZE; -if (heapframes_size > mb->heap_limit) +if (heapframes_size / 1024 > mb->heap_limit) { - if (frame_size > mb->heap_limit ) return PCRE2_ERROR_HEAPLIMIT; - heapframes_size = mb->heap_limit; + PCRE2_SIZE max_size = 1024 * mb->heap_limit; + if (max_size < frame_size) return PCRE2_ERROR_HEAPLIMIT; + heapframes_size = max_size; } /* If an existing frame vector in the match_data block is large enough, we can -use it.Otherwise, free any pre-existing vector and get a new one. */ +use it. Otherwise, free any pre-existing vector and get a new one. */ if (match_data->heapframes_size < heapframes_size) {