Bug 2703 - envz_strip() works incorrectly
Summary: envz_strip() works incorrectly
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-29 03:52 UTC by Sean Lee
Modified: 2019-04-10 07:10 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
envz_split() patch to fix reversed src & dest pointers when invoking memmove(). (204 bytes, patch)
2006-06-02 17:53 UTC, Ryan S. Arnold
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sean Lee 2006-05-29 03:52:14 UTC
The envz_strip () routine in the string/envz.c works incorrectly.

In my opinion after strip "arg1=value1\0arg2\0arg3=value3\0arg4\0" it should
return "arg1=value1\0arg3=value3\0", however, it returns "arg1=value1\0".

After my investigation, I think it is caused by 
  "memmove (entry + entry_len, entry, left);"
which should be
  "memmove (entry, entry + entry_len, left);"
Comment 1 Ryan S. Arnold 2006-06-02 15:10:09 UTC
My tests (on i386 and ppc64) indicate that the problem is a little bit more
bizarre.  The function envz_strip() will successfully process successive strings
in a vector where the string's value isn't NULL.  It will then successfully
strip a successive sequence of strings where the value is null.  It will fail
with a segmentation violation upon encountering the next instance of a string
with a non-NULL value.

#include <stdio.h>
#include <envz.h>
#include <stdlib.h>

int main() {

        size_t size = 0;
        char **argz;
        char *str=malloc(63);
        argz = &str;

        memset(*argz, 0x00, 63);
        memcpy(*argz,
"arg1=value1\0arg2=value2\0arg3\0arg4\0arg5=value5\0arg6\0arg7=value7\0",63);
        size = 29;
        size = 34;
        size = 45;

        write(1, *argz, 63);
        printf("\nsize=%d\n", 63);
        printf("calling envz_strip with size=%d\n", size);
        envz_strip(argz, &size);

        write(1, *argz, size);
        printf("\nsize=%d\n", size);
        argz=0;

        free(str);
        return 0;
}

To test simply comment out two of the three 'size' assignments and witness the
behavior change.  It is successful until 'size' exceeds '34'.

The problem certainly is with the memmove() invocation in envz_strip():

arg1=value1arg2=value2arg3arg4arg5=value5arg6arg7=value7
size=63
calling envz_strip with size=45

Program received signal SIGSEGV, Segmentation fault.
0x10007cfc in _wordcopy_bwd_dest_aligned ()
(gdb) bt
#0  0x10007cfc in _wordcopy_bwd_dest_aligned ()
#1  0x10025604 in memmove ()
#2  0x10008008 in envz_strip ()
#3  0x100003c8 in main ()

I will test the proposed memmove invocation change in envz_strip() and report
the result.

Comment 2 Ryan S. Arnold 2006-06-02 17:53:22 UTC
Created attachment 1062 [details]
envz_split() patch to fix reversed src & dest pointers when invoking memmove().

Here's a better testcase:

#include <stdio.h>
#include <envz.h>
#include <stdlib.h>

int main() {

	size_t size = 0;
	char **argz;
	char *str=malloc(100);
	argz = &str;

	argz_add_sep(argz, &size, "(a=1)", '\0');
	argz_add_sep(argz, &size, "(b=2)", '\0');
	argz_add_sep(argz, &size, "(*)", '\0');
	argz_add_sep(argz, &size, "(*)", '\0');
	argz_add_sep(argz, &size, "(e=5)", '\0');
	argz_add_sep(argz, &size, "(f=)", '\0');
	argz_add_sep(argz, &size, "(*)", '\0');
	argz_add_sep(argz, &size, "(h=8)", '\0');
	argz_add_sep(argz, &size, "(i=9)", '\0');
	argz_add_sep(argz, &size, "(j)", '\0');

	write(1, *argz, size);
	printf("\ncalling envz_strip with size=%d\n", size);
	envz_strip(argz, &size);

	write(1, *argz, size);
	printf("\nsize=%d\n", size);
	argz=0;

	free(str);
	return 0;
}

Sean Lee is correct.  This was a problem of reversed src & dest pointers when
invoking memmove() within envz_split.  Getting a periodic segmentation
violation was unpredictably related to the improper memmove() invocation.  The
attached patch corrects the problem in my tests.
Comment 3 Ryan S. Arnold 2006-06-02 18:36:07 UTC
reference:

http://sourceware.org/ml/libc-alpha/2006-06/msg00002.html
Comment 4 Ryan S. Arnold 2006-06-02 19:57:37 UTC
The segfault related to my first testcase was due to 'size' not falling upon a
\0.  It is an error when the last character in the string passed into
envz_split() is not \0.

The segfault related to my second testcase is due to the memmove parameters
being reversed.
Comment 5 Ulrich Drepper 2006-06-04 04:36:04 UTC
Patch applied.
Comment 6 Sourceware Commits 2006-06-04 16:35:30 UTC
Subject: Bug 2703

CVSROOT:	/cvs/glibc
Module name:	libc
Changes by:	drepper@sources.redhat.com	2006-06-04 16:35:21

Added files:
	string         : bug-envz1.c 

Log message:
	Test for bug BZ #2703.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/string/bug-envz1.c.diff?cvsroot=glibc&r1=NONE&r2=1.1