Bug 478324 - [patch] strace: buffer overflow in function printstr
Summary: [patch] strace: buffer overflow in function printstr
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: strace
Version: 10
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Roland McGrath
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-12-28 04:45 UTC by Scott Tsai
Modified: 2009-11-13 02:34 UTC (History)
4 users (show)

Fixed In Version: 4.5.19-1.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-13 02:33:36 UTC


Attachments (Terms of Use)
printstr-buffer-overflow.patch (371 bytes, patch)
2008-12-28 04:58 UTC, Scott Tsai
no flags Details | Diff

Description Scott Tsai 2008-12-28 04:45:46 UTC
Description of problem:
strace crashed because glibc malloc detected heap corruption. I produced a patch for the heap based buffer overflow in function printstr of util.c

Version-Release number of selected component (if applicable):
strace-4.5.18-1.fc10.x86_64

How reproducible:
always

Steps to Reproduce:
1. export GTK_IM_MODULE=scim ; strace gedit

  
Actual results:
*** glibc detected *** strace: malloc(): memory corruption (fast): 0x0000000001b85460 ***


Expected results:
Not crashing.

Additional info:

Comment 1 Scott Tsai 2008-12-28 04:58:26 UTC
Created attachment 327895 [details]
printstr-buffer-overflow.patch

The original printstr code quoted below could overflow the 'outstr' buffer because:

1. The size allocated for 'outstr' is 4*max_strlen + 6 bytes.
2. string_quote could potentially write 4*(max_strlen + 1) bytes to 'outstr' when 'size' is max_strlen + 1.
3. The strcat(outstr, "...") call would then write beyond 'outstr's allocated space.

void
printstr(struct tcb *tcp, long addr, int len)
{
        static char *str = NULL;
        static char *outstr;
        int size;

        if (!addr) {
                tprintf("NULL");
                return;
        }
        if (!str) {
                if ((str = malloc(max_strlen + 1)) == NULL
                    || (outstr = malloc(4*max_strlen
                                        + sizeof "\"\"...")) == NULL) {
                        fprintf(stderr, "out of memory\n");
                        tprintf("%#lx", addr);
                        return;
                }
        }

        if (len < 0) {
                size = max_strlen + 1;
                if (umovestr(tcp, addr, size, str) < 0) {
                        tprintf("%#lx", addr);
                        return;
                }
        }
        else {
                size = MIN(len, max_strlen + 1);
                if (umoven(tcp, addr, size, str) < 0) {
                        tprintf("%#lx", addr);
                        return;
                }
        }

        if (string_quote(str, outstr, len, size))
                strcat(outstr, "...");

        tprintf("%s", outstr);
}

Comment 2 Denys Vlasenko 2008-12-29 18:04:57 UTC
Current code in cvs tree is different:

void
printstr(struct tcb *tcp, long addr, int len)
{
        static char *str = NULL;
        static char *outstr;
        int size;
        if (!addr) {
                tprintf("NULL");
                return;
        }
        if (!str)
                str = malloc(max_strlen + 1);
        if (!outstr)
                outstr = malloc(4 * max_strlen + sizeof "\"...\"");
        if (!str || !outstr) {
                fprintf(stderr, "out of memory\n");
                tprintf("%#lx", addr);
                return;
        }
        if (len < 0) {
                /*
                 * Treat as a NUL-terminated string: fetch one byte more
                 * because string_quote() quotes one byte less.
                 */
                size = max_strlen + 1;
                str[max_strlen] = '\0';
                if (umovestr(tcp, addr, size, str) < 0) {
                        tprintf("%#lx", addr);
                        return;
                }
        } else {
                size = MIN(len, max_strlen);
                if (umoven(tcp, addr, size, str) < 0) {
                        tprintf("%#lx", addr);
                        return;
                }
        }
        if (string_quote(str, outstr, len, size) &&
            (len < 0 || len > max_strlen))
                strcat(outstr, "...");
        tprintf("%s", outstr);
}

And string_quote() carries the following comment:
/*
 * Quote string `instr' of length `size'
 * Write up to (3 + `size' * 4) bytes to `outstr' buffer.
 * If `len' < 0, treat `instr' as a NUL-terminated string
 * and quote at most (`size' - 1) bytes.
 */

In the case "len < 0" string_quote() will put 3 + max_strlen*4 chars max, including NUL. Then "..." adds 3 more bytes, -> 6 + max_strlen*4.

Do you have a small example which overflows printstr()?

Comment 3 Scott Tsai 2008-12-30 03:12:36 UTC
(In reply to comment #2)
 1. I re-ran my test case against CVS HEAD of trace.cvs.sourceforge.net and the new code indeed works.

 2. Is there a chance to get a new strace build with this fix for Fedora 10? I rely on strace to diagnose all sorts of random sysadmin and development problems and would really like it to not crash.

Comment 4 Andreas Schwab 2009-08-18 16:08:05 UTC
*** Bug 470529 has been marked as a duplicate of this bug. ***

Comment 6 Dmitry V. Levin 2009-09-19 00:22:43 UTC
This issue was fixed in upstream git long time ago, see http://strace.git.sourceforge.net/git/gitweb.cgi?p=strace/strace;h=v4.5.18-17-ga501f14

Comment 7 Fedora Update System 2009-10-21 18:09:25 UTC
strace-4.5.19-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/strace-4.5.19-1.fc10

Comment 8 Fedora Update System 2009-10-21 18:10:04 UTC
strace-4.5.19-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/strace-4.5.19-1.fc11

Comment 9 Fedora Update System 2009-10-27 06:33:06 UTC
strace-4.5.19-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update strace'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-10733

Comment 10 Fedora Update System 2009-10-27 07:13:58 UTC
strace-4.5.19-1.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update strace'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10843

Comment 11 Fedora Update System 2009-11-13 02:33:04 UTC
strace-4.5.19-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 12 Fedora Update System 2009-11-13 02:34:26 UTC
strace-4.5.19-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.