[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20130322221233.GA14200@altlinux.org>
Date: Sat, 23 Mar 2013 02:12:34 +0400
From: "Dmitry V. Levin" <ldv@...linux.org>
To: owl-dev@...ts.openwall.com
Subject: Re: glibc
On Mon, Nov 05, 2012 at 01:23:58AM +0400, Dmitry V. Levin wrote:
> On Sun, Nov 04, 2012 at 11:21:42PM +0400, Vasily Kulikov wrote:
> > On Wed, Oct 31, 2012 at 22:55 +0400, Dmitry V. Levin wrote:
> > > The first one already contains references to getenv, and it goes to libc.a
> > > anyway, so this is not an issue.
> > >
> > > The second one comes from elf/rtld.c that had no references to
> > > getenv before.
> >
> > Sure.
> >
> > > The following patch fixes the linking problem by replacing __strncmp_ia32
> > > with strncmp and removing __GI_strncmp in rtld-strncmp-c.os:
> >
> > This works, thanks! I see you've added it to your git tree only now,
> > did it work before or you just haven't tested the tree on i686?
>
> There is no i686 packages in Sisyphus so I do not usually build glibc for
> i686 and I haven't tested my 2.16+ tree on i686 before.
For the record: the right fix for this issue is to avoid introducing
getenv to rtld, for two reasons:
- getenv uses strncmp, and strncmp is not actually implemented in rtld on
x86-64 because rtld is not used to call strncmp directly or indirectly,
see also http://sourceware.org/bugzilla/show_bug.cgi?id=11120
- locale-related environment sanitation could be implemented in a more
reliable way without relying on getenv.
I've applied the following change to my 2.16+ tree:
diff --git a/elf/dl-environ.c b/elf/dl-environ.c
index cd56998..97bd6bc 100644
--- a/elf/dl-environ.c
+++ b/elf/dl-environ.c
@@ -83,3 +83,33 @@ unsetenv (const char *name)
return 0;
}
+
+void
+unset_unsafe_env (const char *name)
+{
+ char **ep;
+
+ ep = __environ;
+ while (*ep != NULL)
+ {
+ size_t cnt = 0;
+
+ while ((*ep)[cnt] == name[cnt] && name[cnt] != '\0')
+ ++cnt;
+
+ if (name[cnt] == '\0' && (*ep)[cnt] == '=' &&
+ ((*ep)[++cnt] == '.' || strchr((*ep)+cnt, '/')))
+ {
+ /* Found it. Remove this pointer by moving later ones to
+ the front. */
+ char **dp = ep;
+
+ do
+ dp[0] = dp[1];
+ while (*dp++);
+ /* Continue the loop in case NAME appears again. */
+ }
+ else
+ ++ep;
+ }
+}
diff --git a/elf/rtld.c b/elf/rtld.c
index 58f5c1a..a5440ea 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2780,16 +2780,11 @@ process_envvars (enum mode *modep)
}
}
- /* This loop is buggy: it will only check the first occurrence of each
- variable (but will correctly remove all in case of a match). This
- may be a problem if the list is later re-ordered or accessed by an
- application with something other than the glibc getenv(). */
for (nextp = restricted_envvars; *nextp != '\0';
nextp = (char *) rawmemchr (nextp, '\0') + 1)
{
- const char *value = getenv (nextp);
- if (value && (value[0] == '.' || strchr(value, '/')))
- unsetenv (nextp);
+ extern void unset_unsafe_env (const char *) __THROW;
+ unset_unsafe_env (nextp);
}
if (__access ("/etc/suid-debug", F_OK) != 0)
--
ldv
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
