Fix assorted security-grade bugs in the regex engine. All of these problems

are shared with Tcl, since it's their code to begin with, and the patches
have been copied from Tcl 8.5.0.  Problems:

CVE-2007-4769: Inadequate check on the range of backref numbers allows
crash due to out-of-bounds read.
CVE-2007-4772: Infinite loop in regex optimizer for pattern '($|^)*'.
CVE-2007-6067: Very slow optimizer cleanup for regex with a large NFA
representation, as well as crash if we encounter an out-of-memory condition
during NFA construction.

Part of the response to CVE-2007-6067 is to put a limit on the number of
states in the NFA representation of a regex.  This seems needed even though
the within-the-code problems have been corrected, since otherwise the code
could try to use very large amounts of memory for a suitably-crafted regex,
leading to potential DOS by driving the system into swap, activating a kernel
OOM killer, etc.

Although there are certainly plenty of ways to drive the system into effective
DOS with poorly-written SQL queries, these problems seem worth treating as
security issues because many applications might accept regex search patterns
from untrustworthy sources.

Thanks to Will Drewry of Google for reporting these problems.  Patches by Will
Drewry and Tom Lane.

Security: CVE-2007-4769, CVE-2007-4772, CVE-2007-6067

diff --git a/src/backend/regex/regc_color.c b/src/backend/regex/regc_color.c
index 33a6c7920658909fc333a7d3a4ae46321407aeeb..910c869730a7a1cfbdd80aaaf7f9a9bfb696d1ae 100644 (file)
--- a/src/backend/regex/regc_color.c
+++ b/src/backend/regex/regc_color.c
@@ -28,7 +28,7 @@
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
- * $PostgreSQL: pgsql/src/backend/regex/regc_color.c,v 1.5 2005/10/15 02:49:24 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/regex/regc_color.c,v 1.5.4.1 2008/01/03 20:48:49 tgl Exp $
  *
  *
  * Note that there are some incestuous relationships between this code and
@@ -569,12 +569,9 @@ okcolors(struct nfa * nfa,
                        while ((a = cd->arcs) != NULL)
                        {
                                assert(a->co == co);
-                               /* uncolorchain(cm, a); */
-                               cd->arcs = a->colorchain;
+                               uncolorchain(cm, a);
                                a->co = sco;
-                               /* colorchain(cm, a); */
-                               a->colorchain = scd->arcs;
-                               scd->arcs = a;
+                               colorchain(cm, a);
                        }
                        freecolor(cm, co);
                }
@@ -604,7 +601,10 @@ colorchain(struct colormap * cm,
 {
        struct colordesc *cd = &cm->cd[a->co];
 
+       if (cd->arcs != NULL)
+               cd->arcs->colorchainRev = a;
        a->colorchain = cd->arcs;
+       a->colorchainRev = NULL;
        cd->arcs = a;
 }
 
@@ -616,19 +616,22 @@ uncolorchain(struct colormap * cm,
                         struct arc * a)
 {
        struct colordesc *cd = &cm->cd[a->co];
-       struct arc *aa;
+       struct arc *aa = a->colorchainRev;
 
-       aa = cd->arcs;
-       if (aa == a)                            /* easy case */
+       if (aa == NULL)
+       {
+               assert(cd->arcs == a);
                cd->arcs = a->colorchain;
+       }
        else
        {
-               for (; aa != NULL && aa->colorchain != a; aa = aa->colorchain)
-                       continue;
-               assert(aa != NULL);
+               assert(aa->colorchain == a);
                aa->colorchain = a->colorchain;
        }
+       if (a->colorchain != NULL)
+               a->colorchain->colorchainRev = aa;
        a->colorchain = NULL;           /* paranoia */
+       a->colorchainRev = NULL;
 }
 
 /*

diff --git a/src/backend/regex/regc_lex.c b/src/backend/regex/regc_lex.c
index df45701e5aa6580f9c5901b6f7a91b9a9f3bd62c..0297d8ac378c9e280c0f7abcc7da94feb1177f0e 100644 (file)
--- a/src/backend/regex/regc_lex.c
+++ b/src/backend/regex/regc_lex.c
@@ -28,7 +28,7 @@
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
- * $PostgreSQL: pgsql/src/backend/regex/regc_lex.c,v 1.5 2005/10/15 02:49:24 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/regex/regc_lex.c,v 1.5.4.1 2008/01/03 20:48:49 tgl Exp $
  *
  */
 
@@ -846,7 +846,7 @@ lexescape(struct vars * v)
                        if (ISERR())
                                FAILW(REG_EESCAPE);
                        /* ugly heuristic (first test is "exactly 1 digit?") */
-                       if (v->now - save == 0 || (int) c <= v->nsubexp)
+                       if (v->now - save == 0 || ((int) c > 0 && (int) c <= v->nsubexp))
                        {
                                NOTE(REG_UBACKREF);
                                RETV(BACKREF, (chr) c);

diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index fa68d021bc2ad5077bccfc1550600415ff747899..be977978f81e052893eecf528ac58b991b98d4cd 100644 (file)
--- a/src/backend/regex/regc_nfa.c
+++ b/src/backend/regex/regc_nfa.c
@@ -28,7 +28,7 @@
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
- * $PostgreSQL: pgsql/src/backend/regex/regc_nfa.c,v 1.4 2005/10/15 02:49:24 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/regex/regc_nfa.c,v 1.4.4.1 2008/01/03 20:48:49 tgl Exp $
  *
  *
  * One or two things that technically ought to be in here
@@ -60,11 +60,12 @@ newnfa(struct vars * v,
        nfa->nstates = 0;
        nfa->cm = cm;
        nfa->v = v;
+       nfa->size = 0;
        nfa->bos[0] = nfa->bos[1] = COLORLESS;
        nfa->eos[0] = nfa->eos[1] = COLORLESS;
+       nfa->parent = parent;           /* Precedes newfstate so parent is valid. */
        nfa->post = newfstate(nfa, '@');        /* number 0 */
        nfa->pre = newfstate(nfa, '>');         /* number 1 */
-       nfa->parent = parent;
 
        nfa->init = newstate(nfa);      /* may become invalid later */
        nfa->final = newstate(nfa);
@@ -88,6 +89,57 @@ newnfa(struct vars * v,
        return nfa;
 }
 
+/*
+ * TooManyStates - checks if the max states exceeds the compile-time value
+ */
+static int
+TooManyStates(struct nfa *nfa)
+{
+       struct nfa *parent = nfa->parent;
+       size_t          sz = nfa->size;
+
+       while (parent != NULL)
+       {
+               sz = parent->size;
+               parent = parent->parent;
+       }
+       if (sz > REG_MAX_STATES)
+               return 1;
+       return 0;
+}
+
+/*
+ * IncrementSize - increases the tracked size of the NFA and its parents.
+ */
+static void
+IncrementSize(struct nfa *nfa)
+{
+       struct nfa *parent = nfa->parent;
+
+       nfa->size++;
+       while (parent != NULL)
+       {
+               parent->size++;
+               parent = parent->parent;
+       }
+}
+
+/*
+ * DecrementSize - decreases the tracked size of the NFA and its parents.
+ */
+static void
+DecrementSize(struct nfa *nfa)
+{
+       struct nfa *parent = nfa->parent;
+
+       nfa->size--;
+       while (parent != NULL)
+       {
+               parent->size--;
+               parent = parent->parent;
+       }
+}
+
 /*
  * freenfa - free an entire NFA
  */
@@ -122,6 +174,11 @@ newstate(struct nfa * nfa)
 {
        struct state *s;
 
+       if (TooManyStates(nfa))
+       {
+               NERR(REG_ETOOBIG);
+               return NULL;
+       }
        if (nfa->free != NULL)
        {
                s = nfa->free;
@@ -158,6 +215,8 @@ newstate(struct nfa * nfa)
        }
        s->prev = nfa->slast;
        nfa->slast = s;
+       /* track the current size and the parent size */
+       IncrementSize(nfa);
        return s;
 }
 
@@ -220,6 +279,7 @@ freestate(struct nfa * nfa,
        s->prev = NULL;
        s->next = nfa->free;            /* don't delete it, put it on the free list */
        nfa->free = s;
+       DecrementSize(nfa);
 }
 
 /*
@@ -633,6 +693,8 @@ duptraverse(struct nfa * nfa,
        for (a = s->outs; a != NULL && !NISERR(); a = a->outchain)
        {
                duptraverse(nfa, a->to, (struct state *) NULL);
+               if (NISERR())
+                       break;
                assert(a->to->tmp != NULL);
                cparc(nfa, a, s->tmp, a->to->tmp);
        }
@@ -793,6 +855,25 @@ pull(struct nfa * nfa,
                return 1;
        }
 
+       /*
+        * DGP 2007-11-15: Cloning a state with a circular constraint on its list
+        * of outs can lead to trouble [Tcl Bug 1810038], so get rid of them first.
+        */
+       for (a = from->outs; a != NULL; a = nexta)
+       {
+               nexta = a->outchain;
+               switch (a->type)
+               {
+                       case '^':
+                       case '$':
+                       case BEHIND:
+                       case AHEAD:
+                               if (from == a->to)
+                                       freearc(nfa, a);
+                               break;
+               }
+       }
+
        /* first, clone from state if necessary to avoid other outarcs */
        if (from->nouts > 1)
        {
@@ -917,6 +998,29 @@ push(struct nfa * nfa,
                return 1;
        }
 
+       /*
+        * DGP 2007-11-15: Here we duplicate the same protections as appear
+        * in pull() above to avoid troubles with cloning a state with a
+        * circular constraint on its list of ins.  It is not clear whether
+        * this is necessary, or is protecting against a "can't happen".
+        * Any test case that actually leads to a freearc() call here would
+        * be a welcome addition to the test suite.
+        */
+       for (a = to->ins; a != NULL; a = nexta)
+       {
+               nexta = a->inchain;
+               switch (a->type)
+               {
+                       case '^':
+                       case '$':
+                       case BEHIND:
+                       case AHEAD:
+                               if (a->from == to)
+                                       freearc(nfa, a);
+                               break;
+               }
+       }
+
        /* first, clone to state if necessary to avoid other inarcs */
        if (to->nins > 1)
        {
@@ -1039,7 +1143,8 @@ fixempties(struct nfa * nfa,
        do
        {
                progress = 0;
-               for (s = nfa->states; s != NULL && !NISERR(); s = nexts)
+               for (s = nfa->states; s != NULL && !NISERR() &&
+                                s->no != FREESTATE; s = nexts)
                {
                        nexts = s->next;
                        for (a = s->outs; a != NULL && !NISERR(); a = nexta)

diff --git a/src/include/regex/regerrs.h b/src/include/regex/regerrs.h
index 77baf97f4bbdc446f7dae76b6e9f5517eee96468..fc1458b80050e094655767702fe1c4b422faae8f 100644 (file)
--- a/src/include/regex/regerrs.h
+++ b/src/include/regex/regerrs.h
@@ -1,5 +1,5 @@
 /*
- * $PostgreSQL: pgsql/src/include/regex/regerrs.h,v 1.3 2003/11/29 22:41:10 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/regex/regerrs.h,v 1.3.8.1 2008/01/03 20:48:49 tgl Exp $
  */
 
 {
@@ -73,3 +73,7 @@
 {
        REG_BADOPT, "REG_BADOPT", "invalid embedded option"
 },
+
+{
+       REG_ETOOBIG, "REG_ETOOBIG", "nfa has too many states"
+},

diff --git a/src/include/regex/regex.h b/src/include/regex/regex.h
index 9cda1c23ebcf66b149378b293c27ae330bef2ab9..f6cf655a042d3f1f190ade1d38f28f9740ad098e 100644 (file)
--- a/src/include/regex/regex.h
+++ b/src/include/regex/regex.h
@@ -29,7 +29,7 @@
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
- * $PostgreSQL: pgsql/src/include/regex/regex.h,v 1.28 2005/10/15 02:49:46 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/regex/regex.h,v 1.28.4.1 2008/01/03 20:48:49 tgl Exp $
  */
 
 /*
@@ -151,6 +151,7 @@ typedef struct
 #define REG_INVARG     16                      /* invalid argument to regex function */
 #define REG_MIXED      17                      /* character widths of regex and string differ */
 #define REG_BADOPT     18                      /* invalid embedded option */
+#define REG_ETOOBIG    19                      /* nfa has too many states */
 /* two specials for debugging and testing */
 #define REG_ATOI       101                     /* convert error-code name to number */
 #define REG_ITOA       102                     /* convert error-code number to name */

diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h
index 18712b4090d6a55a12ece941cf24c98c42e13392..68384cba0035da1cfce164346db3eafdc9b63c74 100644 (file)
--- a/src/include/regex/regguts.h
+++ b/src/include/regex/regguts.h
@@ -27,7 +27,7 @@
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
- * $PostgreSQL: pgsql/src/include/regex/regguts.h,v 1.5 2005/10/15 02:49:46 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/regex/regguts.h,v 1.5.4.1 2008/01/03 20:48:49 tgl Exp $
  */
 
 
@@ -272,6 +272,7 @@ struct arc
 #define  freechain      outchain
        struct arc *inchain;            /* *to's ins chain */
        struct arc *colorchain;         /* color's arc chain */
+       struct arc *colorchainRev;      /* back-link in color's arc chain */
 };
 
 struct arcbatch
@@ -311,6 +312,9 @@ struct nfa
        struct colormap *cm;            /* the color map */
        color           bos[2];                 /* colors, if any, assigned to BOS and BOL */
        color           eos[2];                 /* colors, if any, assigned to EOS and EOL */
+       size_t          size;                   /* Current NFA size; differs from nstates as
+                                                                * it also counts the number of states created
+                                                                * by children of this state. */
        struct vars *v;                         /* simplifies compile error reporting */
        struct nfa *parent;                     /* parent NFA, if any */
 };
@@ -343,7 +347,12 @@ struct cnfa
 #define ZAPCNFA(cnfa)  ((cnfa).nstates = 0)
 #define NULLCNFA(cnfa) ((cnfa).nstates == 0)
 
-
+/*
+ * Used to limit the maximum NFA size to something sane. [Tcl Bug 1810264]
+ */
+#ifndef REG_MAX_STATES
+#define REG_MAX_STATES 100000
+#endif
 
 /*
  * subexpression tree