tre icon indicating copy to clipboard operation
tre copied to clipboard

Upstream Project Zero security fixes from OS X

Open achivetta opened this issue 10 years ago • 2 comments

Comparing the 10.10.4 and 10.10.5 source to OS X's Libc (on opensource.apple.com) yields the following patch to TRE:

diff -ur Libc-1044.10.1/regex/TRE/lib/regexec.c Libc-1044.40.1/regex/TRE/lib/regexec.c
--- Libc-1044.10.1/regex/TRE/lib/regexec.c  2011-09-29 18:54:25.000000000 -0700
+++ Libc-1044.40.1/regex/TRE/lib/regexec.c  2015-07-09 15:15:19.000000000 -0700
@@ -10,6 +10,10 @@
 #include <config.h>
 #endif /* HAVE_CONFIG_H */

+/* Unset TRE_USE_ALLOCA to avoid using the stack to hold all the state
+   info while running */
+#undef TRE_USE_ALLOCA
+
 #ifdef TRE_USE_ALLOCA
 /* AIX requires this to be the first thing in the file.     */
 #ifndef __GNUC__
diff -ur Libc-1044.10.1/regex/TRE/lib/tre-match-backtrack.c Libc-1044.40.1/regex/TRE/lib/tre-match-backtrack.c
--- Libc-1044.10.1/regex/TRE/lib/tre-match-backtrack.c  2011-09-29 18:54:25.000000000 -0700
+++ Libc-1044.40.1/regex/TRE/lib/tre-match-backtrack.c  2015-07-09 15:15:19.000000000 -0700
@@ -274,7 +274,7 @@

   int num_tags = tnfa->num_tags;
   int touch = 1;
-  char *buf;
+  char *buf = NULL;
   int tbytes;

 #ifdef TRE_MBSTATE
diff -ur Libc-1044.10.1/regex/TRE/lib/tre-match-parallel.c Libc-1044.40.1/regex/TRE/lib/tre-match-parallel.c
--- Libc-1044.10.1/regex/TRE/lib/tre-match-parallel.c   2012-05-03 17:34:12.000000000 -0700
+++ Libc-1044.40.1/regex/TRE/lib/tre-match-parallel.c   2015-07-09 15:15:19.000000000 -0700
@@ -143,7 +143,7 @@
 #endif /* TRE_DEBUG */
   tre_tag_t *tmp_tags = NULL;
   tre_tag_t *tmp_iptr;
-  int tbytes;
+  size_t tbytes;
   int touch = 1;

 #ifdef TRE_MBSTATE
@@ -162,7 +162,7 @@
      everything in a single large block from the stack frame using alloca()
      or with malloc() if alloca is unavailable. */
   {
-    int rbytes, pbytes, total_bytes;
+    size_t rbytes, pbytes, total_bytes;
     char *tmp_buf;
     /* Compute the length of the block we need. */
     tbytes = sizeof(*tmp_tags) * num_tags;
@@ -177,11 +177,11 @@
 #ifdef TRE_USE_ALLOCA
     buf = alloca(total_bytes);
 #else /* !TRE_USE_ALLOCA */
-    buf = xmalloc((unsigned)total_bytes);
+    buf = xmalloc(total_bytes);
 #endif /* !TRE_USE_ALLOCA */
     if (buf == NULL)
       return REG_ESPACE;
-    memset(buf, 0, (size_t)total_bytes);
+    memset(buf, 0, total_bytes);

     /* Get the various pointers within tmp_buf (properly aligned). */
     tmp_tags = (void *)buf;
diff -ur Libc-1044.10.1/regex/TRE/lib/tre-parse.c Libc-1044.40.1/regex/TRE/lib/tre-parse.c
--- Libc-1044.10.1/regex/TRE/lib/tre-parse.c    2011-12-07 18:12:55.000000000 -0800
+++ Libc-1044.40.1/regex/TRE/lib/tre-parse.c    2015-07-09 15:15:19.000000000 -0700
@@ -717,7 +717,7 @@
 static reg_errcode_t
 tre_parse_bracket(tre_parse_ctx_t *ctx, tre_ast_node_t **result)
 {
-  tre_ast_node_t *node;
+  tre_ast_node_t *node = NULL;
   reg_errcode_t status = REG_OK;
   tre_bracket_match_list_t *items;
   int max_i = 32;
@@ -2016,6 +2016,8 @@
            ctx->re++;
            while (ctx->re_end - ctx->re >= 0)
              {
+               if (i == sizeof(tmp))
+               return REG_EBRACE;
                if (ctx->re[0] == CHAR_RBRACE)
                  break;
                if (tre_isxdigit_l(ctx->re[0], ctx->loc))

This patch resolves:

  • https://code.google.com/p/google-security-research/issues/detail?id=428&can=1&q=TRE
  • https://code.google.com/p/google-security-research/issues/detail?id=429&can=1&q=TRE
  • https://code.google.com/p/google-security-research/issues/detail?id=430&can=1&q=TRE

which may also affect upstream TRE.

achivetta avatar Oct 30 '15 16:10 achivetta

One more spot for alloca: tre_tnfa_run_approx(), in tre-match-approx.c.

Artoria2e5 avatar Mar 06 '16 04:03 Artoria2e5

it seems https://bugs.chromium.org/p/project-zero/issues/detail?id=429&can=1&q=TRE&redir=1 is #45, and maybe shoudl be processed there.

428 and 430 have not been reported here yet. i am not sure which chunks apply to what.

anarcat avatar Oct 26 '16 17:10 anarcat

Your patch for the buffer overflow in the wide character parser (the final hunk of the patch) is incorrect. If you pass in "\\x{00000000000000000000000000000000}" (32 zeroes), the final '0' will be stored in tmp[31], then i will be incremented, then the tmp[i] = 0; line just below still overflows tmp. You need to check against sizeof(tmp) - 1, and optionally reduce the size of tmp to something more sensible, like 9.

dag-erling avatar Jul 30 '24 15:07 dag-erling