Bug 478324 - [patch] strace: buffer overflow in function printstr
[patch] strace: buffer overflow in function printstr
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: strace (Show other bugs)
10
All Linux
low Severity medium
: ---
: ---
Assigned To: Roland McGrath
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-27 23:45 EST by Scott Tsai
Modified: 2009-11-12 21:34 EST (History)
4 users (show)

See Also:
Fixed In Version: 4.5.19-1.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-12 21:33:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description Scott Tsai 2008-12-27 23:45:46 EST
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-27 23:58:26 EST
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 13:04:57 EST
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-29 22:12:36 EST
(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 12:08:05 EDT
*** Bug 470529 has been marked as a duplicate of this bug. ***
Comment 6 Dmitry V. Levin 2009-09-18 20:22:43 EDT
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 14:09:25 EDT
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 14:10:04 EDT
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 02:33:06 EDT
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 03:13:58 EDT
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-12 21:33:04 EST
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-12 21:34:26 EST
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.