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

t1disasm: buffer overflow in set_cs_start #4

Closed
nthykier opened this issue Feb 26, 2015 · 6 comments
Closed

t1disasm: buffer overflow in set_cs_start #4

nthykier opened this issue Feb 26, 2015 · 6 comments

Comments

@nthykier
Copy link
Contributor

We received the following report in Debian (https://bugs.debian.org/779274). I have not had time to look at it in details. However, given it was public and it is believed to allow code execution via a crafted file, I decided to forward it immediately.

"""
$ t1asm crash.raw crash.pfb
t1asm: warning: no charstrings found in input file

$ t1disasm crash.pfb /dev/null
Segmentation fault

Backtrace:
#0 ___fprintf_chk (fp=0x6f6f6f6f, flag=1, format=0x804eedc "%.*s") at fprintf_chk.c:30
#1 0x0804d653 in fprintf (__fmt=0x804eedc "%.*s", __stream=) at /usr/include/i386-linux-gnu/bits/stdio2.h:97
#2 eexec_line (line=0xffffd320 "/m", 'o' <repeats 36 times>, "{string currentfile exch readstring pop}executeonly def\n", line_len=, line_len@entry=94) at t1disasm.c:462
#3 0x0804e05e in disasm_output_binary (data=0xffffd320 "/m", 'o' <repeats 36 times>, "{string currentfile exch readstring pop}executeonly def\n", len=94) at t1disasm.c:595
#4 0x0804cf67 in process_pfb (ifp=0x80531c0, ifp_filename=0xffffd9ff "crash.pfb", fr=0xffffd760) at t1lib.c:295
#5 0x08048f41 in main (argc=3, argv=0xffffd834) at t1disasm.c:770

This happened because set_cs_start overwrote the file pointer with data from the disassembled file.

I believe the bug can be exploited for code execution, at least on systems that don't have executable space protection.

This bug was found using American fuzzy lop:
http://lcamtuf.coredump.cx/afl/
"""

The file contents of crash.raw:

currentfile eexec
/moooooooooooooooooooooooooooooooooooo{string currentfile exch readstring pop}executeonly def
@nthykier
Copy link
Contributor Author

I had no luck reproducing the issue with the provided file, but I /can/ produce an overrun of cs_start with the following crash.raw file:

currentfile eexec
/mooooooooo{string currentfile exch readstring pop}executeonly def

The following debug code was used:

index 5def559..b917413 100644
--- a/t1disasm.c
+++ b/t1disasm.c
@@ -123,9 +123,20 @@ set_cs_start(char *line)
       while (!isspace(*q) && *q != '{')
        *r++ = *q++;
       *r = '\0';
+      if (r - cs_start >= 0 && r - cs_start < (signed)sizeof(cs_start)) {
+        fprintf(stderr, "r diff: %ld\n", r - cs_start);
+      } else {
+        fprintf(stderr, "r overran cs_start\n");
+        abort();
+      }
+    } else {
+      fprintf(stderr, "q is null\n");
     }
     *p = 's';                                    /* repair line[] */
+  } else {
+    fprintf(stderr, "p is null\n");
   }
+  fprintf(stderr, "No overrun\n");
 }

 /* Subroutine to output strings. */

@nthykier
Copy link
Contributor Author

The following patch is a possible solution using "fatal_error" to stop processing:

diff --git a/t1disasm.c b/t1disasm.c
index 5def559..889097e 100644
--- a/t1disasm.c
+++ b/t1disasm.c
@@ -118,10 +118,14 @@ set_cs_start(char *line)
     *p = '\0';                                   /* damage line[] */
     q = strrchr(line, '/');
     if (q) {
+      char *limit = cs_start + sizeof(cs_start);
       r = cs_start;
       ++q;
-      while (!isspace(*q) && *q != '{')
+      while (!isspace(*q) && *q != '{' && r < limit)
        *r++ = *q++;
+      if (r == limit) {
+        fatal_error("disassembly error: cs_start limit exceeded");
+      }
       *r = '\0';
     }
     *p = 's';                                    /* repair line[] */

Though it could possibly use a better error message.

@kohler
Copy link
Owner

kohler commented Feb 26, 2015

Thanks, I believe this is fixed in 6b9d1aa

@nthykier
Copy link
Contributor Author

nthykier commented Mar 1, 2015

Thanks, looks ok at first glance. I will try to run afl-fuzz on that version soon. For Debian/Jessie, I decided to stick with the "simple" limit check (since Debian in a deep freeze right). I also increased the cs_start buffer to 1024, which (hopefully) be "more than enough" for Jessie.

I will let you know, if fuzzing finds anything. :)

@nthykier nthykier closed this as completed Mar 1, 2015
@nthykier
Copy link
Contributor Author

nthykier commented Mar 7, 2015

The fuzzer has run a couple of cycles now without finding anything. :) I am hoping it stays that way.

@carnil
Copy link

carnil commented May 23, 2015

Hi

FTR, this issue has been assigned CVE-2015-3905, see http://www.openwall.com/lists/oss-security/2015/05/22/10

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

No branches or pull requests

3 participants