Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream Project Zero security fixes from OS X #37

Closed
achivetta opened this issue Oct 30, 2015 · 3 comments · Fixed by #108
Closed

Upstream Project Zero security fixes from OS X #37

achivetta opened this issue Oct 30, 2015 · 3 comments · Fixed by #108

Comments

@achivetta
Copy link

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:

which may also affect upstream TRE.

@Artoria2e5
Copy link
Contributor

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

@anarcat
Copy link

anarcat commented Oct 26, 2016

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.

@dag-erling
Copy link
Collaborator

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 added a commit to dag-erling/tre that referenced this issue Jul 30, 2024
This fixes a buffer overflow which was reported in laurikari#37.
dag-erling added a commit to dag-erling/tre that referenced this issue Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants