From ae4e6261e5681658f88a0dff8eb2d60112bd7e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= Date: Sat, 27 Nov 2021 08:49:31 -0800 Subject: [PATCH] match: avoid crash if subject NULL and PCRE2_ZERO_TERMINATED (#53) * pcre2_match: avoid crash if subject NULL and PCRE2_ZERO_TERMINATED When length of subject is PCRE2_ZERO_TERMINATED strlen is used to calculate its size, which will trigger a crash if subject is also NULL. Move the NULL check before strlen on it would be used, and make sure or dependent variables are set after the NULL validation as well. While at it, fix a typo in a debug flag in the same file, which is otherwise unrelated and make sure the full section of constrain checks can be identified clearly using the leading comment alone. * pcre2_dfa_match: avoid crash if subject NULL and PCRE2_ZERO_TERMINATED When length of subject is PCRE2_ZERO_TERMINATED strlen is used to calculate its size, which will trigger a crash if subject is also NULL. Move the NULL check before the detection for subject sizes to avoid this issue. * pcre2_substitute: avoid crash if subject or replacement are NULL The underlying pcre2_match() function will validate the subject if needed, but will crash when length is PCRE2_ZERO_TERMINATED or if subject == NULL and pcre2_match() is not being called because match_data was provided. The replacement parameter is missing NULL checks, and so currently allows for an equivalent response to "" if rlength == 0. Restrict all other cases to avoid strlen(NULL) crashes in the same way that is done for subject, but also make sure to reject invalid length values as early as possible. --- doc/pcre2api.3 | 4 +++- src/pcre2_dfa_match.c | 12 +++++------- src/pcre2_match.c | 20 ++++++++++---------- src/pcre2_substitute.c | 15 ++++++++++----- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/doc/pcre2api.3 b/doc/pcre2api.3 index 1caccbb..fe84fa4 100644 --- a/doc/pcre2api.3 +++ b/doc/pcre2api.3 @@ -3649,7 +3649,9 @@ needed is returned via \fIoutlengthptr\fP. Note that this does not happen by default. .P PCRE2_ERROR_NULL is returned if PCRE2_SUBSTITUTE_MATCHED is set but the -\fImatch_data\fP argument is NULL. +\fImatch_data\fP argument is NULL or if the \fIsubject\fP or \fIreplacement\fP +arguments are NULL. For backward compatibility reasons an exception is made for +the \fIreplacement\fP argument if the \fIrlength\fP argument is also 0. .P PCRE2_ERROR_BADREPLACEMENT is used for miscellaneous syntax errors in the replacement string, with more particular errors being PCRE2_ERROR_BADREPESCAPE diff --git a/src/pcre2_dfa_match.c b/src/pcre2_dfa_match.c index 060dc76..a97e071 100644 --- a/src/pcre2_dfa_match.c +++ b/src/pcre2_dfa_match.c @@ -3285,8 +3285,11 @@ rws->next = NULL; rws->size = RWS_BASE_SIZE; rws->free = RWS_BASE_SIZE - RWS_ANCHOR_SIZE; -/* A length equal to PCRE2_ZERO_TERMINATED implies a zero-terminated -subject string. */ +/* Plausibility checks */ + +if ((options & ~PUBLIC_DFA_MATCH_OPTIONS) != 0) return PCRE2_ERROR_BADOPTION; +if (re == NULL || subject == NULL || workspace == NULL || match_data == NULL) + return PCRE2_ERROR_NULL; if (length == PCRE2_ZERO_TERMINATED) { @@ -3294,11 +3297,6 @@ if (length == PCRE2_ZERO_TERMINATED) was_zero_terminated = 1; } -/* Plausibility checks */ - -if ((options & ~PUBLIC_DFA_MATCH_OPTIONS) != 0) return PCRE2_ERROR_BADOPTION; -if (re == NULL || subject == NULL || workspace == NULL || match_data == NULL) - return PCRE2_ERROR_NULL; if (wscount < 20) return PCRE2_ERROR_DFA_WSSIZE; if (start_offset > length) return PCRE2_ERROR_BADOFFSET; diff --git a/src/pcre2_match.c b/src/pcre2_match.c index f28cdbb..ea8ca5d 100644 --- a/src/pcre2_match.c +++ b/src/pcre2_match.c @@ -49,7 +49,7 @@ POSSIBILITY OF SUCH DAMAGE. /* #define DEBUG_SHOW_OPS */ /* #define DEBUG_SHOW_RMATCH */ -#ifdef DEBUG_FRAME_DISPLAY +#ifdef DEBUG_FRAMES_DISPLAY #include #endif @@ -6129,8 +6129,8 @@ PCRE2_UCHAR req_cu2 = 0; PCRE2_SPTR bumpalong_limit; PCRE2_SPTR end_subject; PCRE2_SPTR true_end_subject; -PCRE2_SPTR start_match = subject + start_offset; -PCRE2_SPTR req_cu_ptr = start_match - 1; +PCRE2_SPTR start_match; +PCRE2_SPTR req_cu_ptr; PCRE2_SPTR start_partial; PCRE2_SPTR match_partial; @@ -6170,9 +6170,14 @@ PCRE2_SPTR stack_frames_vector[START_FRAMES_SIZE/sizeof(PCRE2_SPTR)] PCRE2_KEEP_UNINITIALIZED; mb->stack_frames = (heapframe *)stack_frames_vector; -/* A length equal to PCRE2_ZERO_TERMINATED implies a zero-terminated -subject string. */ +/* Plausibility checks */ +if ((options & ~PUBLIC_MATCH_OPTIONS) != 0) return PCRE2_ERROR_BADOPTION; +if (code == NULL || subject == NULL || match_data == NULL) + return PCRE2_ERROR_NULL; + +start_match = subject + start_offset; +req_cu_ptr = start_match - 1; if (length == PCRE2_ZERO_TERMINATED) { length = PRIV(strlen)(subject); @@ -6180,11 +6185,6 @@ if (length == PCRE2_ZERO_TERMINATED) } true_end_subject = end_subject = subject + length; -/* Plausibility checks */ - -if ((options & ~PUBLIC_MATCH_OPTIONS) != 0) return PCRE2_ERROR_BADOPTION; -if (code == NULL || subject == NULL || match_data == NULL) - return PCRE2_ERROR_NULL; if (start_offset > length) return PCRE2_ERROR_BADOFFSET; /* Check that the first field in the block is the magic number. */ diff --git a/src/pcre2_substitute.c b/src/pcre2_substitute.c index 981a106..7aefb60 100644 --- a/src/pcre2_substitute.c +++ b/src/pcre2_substitute.c @@ -260,6 +260,12 @@ PCRE2_UNSET, so as not to imply an offset in the replacement. */ if ((options & (PCRE2_PARTIAL_HARD|PCRE2_PARTIAL_SOFT)) != 0) return PCRE2_ERROR_BADOPTION; +/* Validate length and find the end of the replacement. */ +if (replacement == NULL && rlength > 0) return PCRE2_ERROR_NULL; +else if (rlength == PCRE2_ZERO_TERMINATED) + rlength = PRIV(strlen)(replacement); +repend = replacement + rlength; + /* Check for using a match that has already happened. Note that the subject pointer in the match data may be NULL after a no-match. */ @@ -292,6 +298,7 @@ else if (use_existing_match) (pcre2_general_context *)mcontext; int pairs = (code->top_bracket + 1 < match_data->oveccount)? code->top_bracket + 1 : match_data->oveccount; + if (subject == NULL) return PCRE2_ERROR_NULL; internal_match_data = pcre2_match_data_create(match_data->oveccount, gcontext); if (internal_match_data == NULL) return PCRE2_ERROR_NOMEMORY; @@ -312,11 +319,9 @@ scb.input = subject; scb.output = (PCRE2_SPTR)buffer; scb.ovector = ovector; -/* Find lengths of zero-terminated strings and the end of the replacement. */ - -if (length == PCRE2_ZERO_TERMINATED) length = PRIV(strlen)(subject); -if (rlength == PCRE2_ZERO_TERMINATED) rlength = PRIV(strlen)(replacement); -repend = replacement + rlength; +/* Find lengths of zero-terminated subject */ +if (length == PCRE2_ZERO_TERMINATED) + length = subject? PRIV(strlen)(subject) : 0; /* Check UTF replacement string if necessary. */