From 0d996908097a6a694ea7c3c76f7cec0c92bdc70e Mon Sep 17 00:00:00 2001
From: liuwei1031 <46661762+liuwei1031@users.noreply.github.com>
Date: Wed, 31 Jul 2019 16:55:07 +0800
Subject: [PATCH] fix several security bugs reported by security team (#18831)

* fix security issue, test=develop

* bug fix, test=develop

* throw an exception when null pointer data with non-zero length PaddleBuf is passed, test=develop
---
 paddle/fluid/inference/api/api.cc       | 11 +++++++++--
 paddle/fluid/operators/detection/gpc.cc |  7 ++++++-
 paddle/fluid/string/piece.cc            | 22 +++++++++++++++++-----
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/paddle/fluid/inference/api/api.cc b/paddle/fluid/inference/api/api.cc
index fc2d7b48c2a..ec659f1cfc6 100644
--- a/paddle/fluid/inference/api/api.cc
+++ b/paddle/fluid/inference/api/api.cc
@@ -54,8 +54,15 @@ PaddleBuf &PaddleBuf::operator=(const PaddleBuf &other) {
     memory_owned_ = other.memory_owned_;
   } else {
     Resize(other.length());
-    PADDLE_ENFORCE(!(other.length() > 0 && other.data() == nullptr));
-    memcpy(data_, other.data(), other.length());
+    // if other.length() == 0 or other.data() == nullptr, then the memcpy
+    // behavior is undefined
+    if (other.length() && other.data())
+      memcpy(data_, other.data(), other.length());
+    else if (other.length())
+      PADDLE_THROW(
+          "Invalid argument, null pointer data with length %u is passed",
+          other.length());
+
     length_ = other.length();
     memory_owned_ = true;
   }
diff --git a/paddle/fluid/operators/detection/gpc.cc b/paddle/fluid/operators/detection/gpc.cc
index f46aaf7d0a7..b46d231d0ff 100644
--- a/paddle/fluid/operators/detection/gpc.cc
+++ b/paddle/fluid/operators/detection/gpc.cc
@@ -532,6 +532,7 @@ static int count_contours(polygon_node *polygon) {
 }
 
 static void add_left(polygon_node *p, double x, double y) {
+  PADDLE_ENFORCE_NOT_NULL(p);
   vertex_node *nv = NULL;
 
   /* Create a new vertex node and set its fields */
@@ -587,6 +588,7 @@ static void add_right(polygon_node *p, double x, double y) {
 }
 
 static void merge_right(polygon_node *p, polygon_node *q, polygon_node *list) {
+  PADDLE_ENFORCE_NOT_NULL(p);
   polygon_node *target = NULL;
 
   /* Label contour as external */
@@ -662,6 +664,7 @@ void add_vertex(vertex_node **t, double x, double y) {
 }
 
 void gpc_vertex_create(edge_node *e, int p, int s, double x, double y) {
+  PADDLE_ENFORCE_NOT_NULL(e);
   add_vertex(&(e->outp[p]->v[s]), x, y);
   e->outp[p]->active++;
 }
@@ -1014,6 +1017,7 @@ void gpc_polygon_clip(gpc_op op, gpc_polygon *subj, gpc_polygon *clip,
     e0 = aet;
     e1 = aet;
     /* Set up bundle fields of first edge */
+    PADDLE_ENFORCE_NOT_NULL(aet);
     aet->bundle[ABOVE][aet->type] = (aet->top.y != yb);
     aet->bundle[ABOVE][!aet->type] = 0;
     aet->bstate[ABOVE] = UNBUNDLED;
@@ -1646,6 +1650,7 @@ void gpc_tristrip_clip(gpc_op op, gpc_polygon *subj, gpc_polygon *clip,
     e1 = aet;
 
     /* Set up bundle fields of first edge */
+    PADDLE_ENFORCE_NOT_NULL(aet);
     aet->bundle[ABOVE][aet->type] = (aet->top.y != yb);
     aet->bundle[ABOVE][!aet->type] = 0;
     aet->bstate[ABOVE] = UNBUNDLED;
@@ -1782,7 +1787,7 @@ void gpc_tristrip_clip(gpc_op op, gpc_polygon *subj, gpc_polygon *clip,
                 }
                 new_tristrip(&tlist, cf, cf->xb, yb);
               }
-              edge->outp[ABOVE] = cf->outp[ABOVE];
+              if (cf) edge->outp[ABOVE] = cf->outp[ABOVE];
               gpc_vertex_create(edge, ABOVE, RIGHT, xb, yb);
               break;
             case ILI:
diff --git a/paddle/fluid/string/piece.cc b/paddle/fluid/string/piece.cc
index 8e8cfb0e913..e60eb0d614e 100644
--- a/paddle/fluid/string/piece.cc
+++ b/paddle/fluid/string/piece.cc
@@ -20,6 +20,13 @@
 #include <iosfwd>
 #include <stdexcept>
 
+#define CHAR_POINTER_CMP(a, b) \
+  do {                         \
+    if (!a && !b) return 0;    \
+    if (!a) return -1;         \
+    if (!b) return 1;          \
+  } while (0)
+
 namespace paddle {
 namespace string {
 
@@ -40,6 +47,7 @@ char Piece::operator[](size_t n) const {
 }
 
 int Compare(Piece a, Piece b) {
+  CHAR_POINTER_CMP(a.data(), b.data());
   const size_t min_len = (a.len() < b.len()) ? a.len() : b.len();
   int r = memcmp(a.data(), b.data(), min_len);
   if (r == 0) {
@@ -52,8 +60,10 @@ int Compare(Piece a, Piece b) {
 }
 
 bool operator==(Piece x, Piece y) {
-  return ((x.len() == y.len()) &&
-          (x.data() == y.data() || memcmp(x.data(), y.data(), x.len()) == 0));
+  return (!x.len() && !y.len()) ? true
+                                : ((x.len() == y.len()) &&
+                                   (x.data() == y.data() ||
+                                    memcmp(x.data(), y.data(), x.len()) == 0));
 }
 
 bool operator!=(Piece x, Piece y) { return !(x == y); }
@@ -65,12 +75,14 @@ bool operator<=(Piece x, Piece y) { return Compare(x, y) <= 0; }
 bool operator>=(Piece x, Piece y) { return Compare(x, y) >= 0; }
 
 bool HasPrefix(Piece s, Piece x) {
-  return ((s.len() >= x.len()) && (memcmp(s.data(), x.data(), x.len()) == 0));
+  return !x.len() ? true : ((s.len() >= x.len()) &&
+                            (memcmp(s.data(), x.data(), x.len()) == 0));
 }
 
 bool HasSuffix(Piece s, Piece x) {
-  return ((s.len() >= x.len()) &&
-          (memcmp(s.data() + (s.len() - x.len()), x.data(), x.len()) == 0));
+  return !x.len() ? true : ((s.len() >= x.len()) &&
+                            (memcmp(s.data() + (s.len() - x.len()), x.data(),
+                                    x.len()) == 0));
 }
 
 Piece SkipPrefix(Piece s, size_t n) {
-- 
GitLab