public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Liming Gao <liming.gao@intel.com>
To: edk2-devel@lists.01.org
Cc: Dongao Guo <dongao.guo@intel.com>
Subject: [Patch 4/4] MdeModulePkg/RegularExpressionDxe: Add null pointer check
Date: Thu, 11 Oct 2018 14:57:03 +0800	[thread overview]
Message-ID: <1539241023-2864-5-git-send-email-liming.gao@intel.com> (raw)
In-Reply-To: <1539241023-2864-1-git-send-email-liming.gao@intel.com>

From: Dongao Guo <dongao.guo@intel.com>

There are five check not necessary in logic ,just for pass static
analysis. More detail please refer to comment in code.
And the rest changes are all accepted by owner, they are reasonable.

Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dongao Guo <dongao.guo@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---
 .../RegularExpressionDxe/Oniguruma/regcomp.c       |  8 ++++--
 .../RegularExpressionDxe/Oniguruma/regexec.c       | 31 +++++++++++++++++++---
 .../RegularExpressionDxe/Oniguruma/regparse.c      |  1 +
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c
index cc06178..5cefafc 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c
@@ -3546,8 +3546,12 @@ expand_case_fold_string_alt(int item_num, OnigCaseFoldCodeItem items[], UChar *p
     if (IS_NULL(an)) {
       goto mem_err2;
     }
-
-    if (items[i].byte_len != slen) {
+    //The NULL pointer check is not necessary. It is added just for pass static
+    //analysis. When condition "items[i].byte_len != slen" is true, "varlen = 1" 
+    //in line 3503 will be reached ,so that "if (IS_NULL(var_anode)) return ONIGERR_MEMORY" 
+    //in line 3510 will be executed, so the null pointer has been checked before 
+    //deferenced in line 3584.
+    if (items[i].byte_len != slen && IS_NOT_NULL(var_anode)) {
       Node *rem;
       UChar *q = p + items[i].byte_len;
 
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regexec.c b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regexec.c
index 26e7a31..7f5bb8b 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regexec.c
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regexec.c
@@ -4088,6 +4088,11 @@ slow_search_backward(OnigEncoding enc, UChar* target, UChar* target_end,
     s = ONIGENC_LEFT_ADJUST_CHAR_HEAD(enc, adjust_text, s);
 
   while (s >= text) {
+    //if text is not null,the logic is correct.
+    //this function is only invoked by backward_search_range,parameter text come 
+    //from range, which is checked by "if (range == 0) goto fail" in line 4512
+    //so the check is just for passing static analysis.
+    if(IS_NULL(s))break;
     if (*s == *target) {
       p = s + 1;
       t = target + 1;
@@ -4298,6 +4303,11 @@ map_search_backward(OnigEncoding enc, UChar map[],
   const UChar *s = text_start;
 
   while (s >= text) {
+    //if text is not null,the logic is correct.
+    //this function is only invoked by backward_search_range,parameter text come 
+    //from range, which is checked by "if (range == 0) goto fail" in line 4512
+    //so the check is just for passing static analysis.
+    if(IS_NULL(s))break;
     if (map[*s]) return (UChar* )s;
 
     s = onigenc_get_prev_char_head(enc, adjust_text, s);
@@ -4499,7 +4509,7 @@ backward_search_range(regex_t* reg, const UChar* str, const UChar* end,
                       UChar** low, UChar** high)
 {
   UChar *p;
-
+  if (range == 0) goto fail;
   range += reg->dmin;
   p = s;
 
@@ -4550,7 +4560,7 @@ backward_search_range(regex_t* reg, const UChar* str, const UChar* end,
       case ANCHOR_BEGIN_LINE:
         if (!ON_STR_BEGIN(p)) {
           prev = onigenc_get_prev_char_head(reg->enc, str, p);
-          if (!ONIGENC_IS_MBC_NEWLINE(reg->enc, prev, end)) {
+          if (IS_NOT_NULL(prev) && !ONIGENC_IS_MBC_NEWLINE(reg->enc, prev, end)) {
             p = prev;
             goto retry;
           }
@@ -4739,10 +4749,15 @@ onig_search_with_param(regex_t* reg, const UChar* str, const UChar* end,
       }
     }
     else if (reg->anchor & ANCHOR_SEMI_END_BUF) {
+
       UChar* pre_end = ONIGENC_STEP_BACK(reg->enc, str, end, 1);
 
       max_semi_end = (UChar* )end;
-      if (ONIGENC_IS_MBC_NEWLINE(reg->enc, pre_end, end)) {
+      // only when str > end, pre_end will be null
+      // line 4659 "if (start > end || start < str) goto mismatch_no_msa" 
+      // will guarantee str alwayls less than end
+      // so pre_end won't be null,this check is just for passing staic analysis
+      if (IS_NOT_NULL(pre_end) && ONIGENC_IS_MBC_NEWLINE(reg->enc, pre_end, end)) {
         min_semi_end = pre_end;
 
 #ifdef USE_CRNL_AS_LINE_TERMINATOR
@@ -4891,6 +4906,16 @@ onig_search_with_param(regex_t* reg, const UChar* str, const UChar* end,
             MATCH_AND_RETURN_CHECK(orig_start);
             s = prev;
           }
+          // if range is not null,the check is not necessary.
+          // the range is actually the pointer of the end of the matched string 
+          // or assigned by "range = str" in line 4708. In RegularExpressionMatch 
+          // protocol, the matched string is the parameter String. And str in 
+          // line 4708 is the String,too. and the range is calculated from 
+          // "Start + onigenc_str_bytelen_null (CHAR16_ENCODING, Start)" in 
+          // line 146 in RegularExpressionDxe.c. RegularExpressionMatch ensure 
+          // the String is not null,So in both situation, the range can not be NULL.
+          // This check is just for passing static analysis.
+          if(IS_NULL(s))break;
         } while (s >= range);
         goto mismatch;
       }
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c
index c71ae5f..0a2d6f8 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c
@@ -1572,6 +1572,7 @@ onig_set_callout_of_name(OnigEncoding enc, OnigCalloutType callout_type,
     fe->arg_types[i] = arg_types[i];
   }
   for (i = arg_num - opt_arg_num, j = 0; i < arg_num; i++, j++) {
+    if(IS_NULL(opt_defaults))return ONIGERR_INVALID_ARGUMENT;
     if (fe->arg_types[i] == ONIG_TYPE_STRING) {
       OnigValue* val = opt_defaults + j;
       UChar* ds = onigenc_strdup(enc, val->s.start, val->s.end);
-- 
2.10.0.windows.1



      parent reply	other threads:[~2018-10-11  6:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-11  6:56 [Patch 0/4] MdeModulePkg/RegularExpressionDxe: Miss null pointer check Liming Gao
2018-10-11  6:57 ` [Patch 1/4] " Liming Gao
2018-10-11  6:57 ` [Patch 2/4] " Liming Gao
2018-10-11  6:57 ` [Patch 3/4] " Liming Gao
2018-10-11  6:57 ` Liming Gao [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1539241023-2864-5-git-send-email-liming.gao@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox