[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LNX.2.20.1601041403590.15644@monopod.intra.ispras.ru>
Date: Mon, 4 Jan 2016 14:56:26 +0300 (MSK)
From: Alexander Monakov <amonakov@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix use of pointer after free in unsetenv
On Mon, 4 Jan 2016, Alexander Cherepanov wrote:
> This depends on whether our __env_map[j] could be 0. The condition
> "__env_map[j]" in the previous loop hints that it could. Then it should be
> something like this:
>
> if (__env_map[j]) {
> free (__env_map[j]);
> do __env_map[j] = __env_map[j+1];
> while (__env_map[++j]);
> }
True, but it wouldn't solve the problem in its entirety. There's a similar
issue further down (line 26 atm), where __environ[i] is tested in a loop.
However, if we executed free(__env_map[j]), then __env_map[j]==__environ[i]
had held. Thus, entering the loop invokes the same UB.
To me the implementation looks weird due to how it restarts scanning __environ
with 'goto again' from position 0 instead of current position. I can propose
the following rewrite (untested):
for (i=0; __environ[i]; i++) {
char *e = __environ[i];
if (!memcmp(name, e, l) && e[l] == '=') {
for (j=i--; __environ[j]; j++)
__environ[j] = __environ[j+1];
if (__env_map) {
for (j=0; __env_map[j] && __env_map[j] != e; j++);
if (__env_map[j]) {
free(__env_map[j]);
do __env_map[j] = __env_map[j+1];
while (__env_map[++j]);
}
}
}
}
Alexadner
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.
