[PATCH] env: avoid leaving dangling pointers in __env_map

[PATCH] env: avoid leaving dangling pointers in __env_map

[Alexander Monakov]

This is the minimal fix for __putenv leaving a pointer to freed heap
storage in __env_map array, which could later on lead to errors such
as double-free.
---
This was discovered by code inspection after Rich asked me to develop
testcases for another environment-related patch in a recent thread.
There's another known issue due to logic errors in surrounding code
(a memory leak due to putenv never freeing storage allocated by preceding
setenv), but that is planned to be addressed with a patch overhauling the
implementation.

Thanks.
Alexander

 src/env/putenv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/env/putenv.c b/src/env/putenv.c
index 4042869..7153042 100644
--- a/src/env/putenv.c
+++ b/src/env/putenv.c
@@ -30,6 +30,7 @@ int __putenv(char *s, int a)
 				}
 			} else {
 				free(__env_map[j]);
+				__env_map[j] = s;
 			}
 		}
 	}
-- 
2.1.3

[libc-test][PATCH] add putenv double-free regression test

[Alexander Monakov]

---
I see the musl patch has been committed, so here's the corresponding libc-test
regression test.

It's pretty bare-bones since it just relies on libc's built-in double-free
detection.

Also it's pretty lazy to use _GNU_SOURCE ftm to get putenv; I'm not sure
what's the expected style in libc-test for such things though.
src/functional/env.c also uses _GNU_SOURCE.

 src/regression/putenv-doublefree.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 src/regression/putenv-doublefree.c

diff --git a/src/regression/putenv-doublefree.c b/src/regression/putenv-doublefree.c
new file mode 100644
index 0000000..f3a1f6c
--- /dev/null
+++ b/src/regression/putenv-doublefree.c
@@ -0,0 +1,16 @@
+// commit 9543656cc32fda48fc463f332ee20e91eed2b768 2016-03-06
+// __putenv could be confused into freeing storage that does not belong to the implementation
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <string.h>
+
+int main(void)
+{
+	setenv("A", "1", 1);
+	setenv("A", "2", 1);
+	char *c = strdup("A=3");
+	putenv(c);
+	setenv("A", "4", 1);
+	free(c);
+	return 0;
+}
-- 
2.1.3

Re: [libc-test][PATCH] add putenv double-free regression test

[Szabolcs Nagy]

* Alexander Monakov <amonakov@ispras.ru> [2016-03-06 22:41:14 +0300]:
> ---
> I see the musl patch has been committed, so here's the corresponding libc-test
> regression test.
> 
> It's pretty bare-bones since it just relies on libc's built-in double-free
> detection.
> 
> Also it's pretty lazy to use _GNU_SOURCE ftm to get putenv; I'm not sure
> what's the expected style in libc-test for such things though.
> src/functional/env.c also uses _GNU_SOURCE.
> 

thanks, applied.

used _XOPEN_SOURCE that is at least a standard.

>  src/regression/putenv-doublefree.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 src/regression/putenv-doublefree.c
> 
> diff --git a/src/regression/putenv-doublefree.c b/src/regression/putenv-doublefree.c
> new file mode 100644
> index 0000000..f3a1f6c
> --- /dev/null
> +++ b/src/regression/putenv-doublefree.c
> @@ -0,0 +1,16 @@
> +// commit 9543656cc32fda48fc463f332ee20e91eed2b768 2016-03-06
> +// __putenv could be confused into freeing storage that does not belong to the implementation
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <string.h>
> +
> +int main(void)
> +{
> +	setenv("A", "1", 1);
> +	setenv("A", "2", 1);
> +	char *c = strdup("A=3");
> +	putenv(c);
> +	setenv("A", "4", 1);
> +	free(c);
> +	return 0;
> +}
> -- 
> 2.1.3