[LTP] [RFC PATCH 0/2] CVE-2018-1000001 (glibc)

[LTP] [RFC PATCH 0/2] CVE-2018-1000001 (glibc)

[Petr Vorel]

Hi,

sending simple test of CVE-2018-1000001 (vulnerability in glibc), based
on test in glibc [1] contributed by Dmitry V. Levin.

[1] https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=52a713fdd0a30e1bd79818e2e3c4ab44ddca1a94;hp=249a5895f120b13290a372a49bb4b499e749806f

Petr Vorel (2):
  lib: Add SAFE_CHROOT(path) macro
  cve/cve-2018-1000001: Add Realpath Buffer Underflow test

 include/safe_macros_fn.h         |  3 ++
 include/tst_safe_macros.h        |  5 ++-
 lib/safe_macros.c                | 15 +++++++++
 testcases/cve/cve-2018-1000001.c | 66 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 testcases/cve/cve-2018-1000001.c

-- 
2.15.1

[LTP] [RFC PATCH 1/2] lib: Add SAFE_CHROOT(path) macro

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/safe_macros_fn.h  |  3 +++
 include/tst_safe_macros.h |  5 ++++-
 lib/safe_macros.c         | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h
index 3df952811..9b11801a4 100644
--- a/include/safe_macros_fn.h
+++ b/include/safe_macros_fn.h
@@ -30,6 +30,9 @@ char* safe_basename(const char *file, const int lineno,
 int safe_chdir(const char *file, const int lineno,
                void (*cleanup_fn)(void), const char *path);
 
+int safe_chroot(const char *file, const int lineno,
+               void (*cleanup_fn)(void), const char *path);
+
 int safe_close(const char *file, const int lineno,
                void (*cleanup_fn)(void), int fildes);
 
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 06bff13c7..66678dd76 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2015 Linux Test Project
+ * Copyright (c) 2010-2018 Linux Test Project
  * Copyright (c) 2011-2015 Cyril Hrubis <chrubis@suse.cz>
  *
  * This program is free software: you can redistribute it and/or modify
@@ -36,6 +36,9 @@
 #define SAFE_BASENAME(path) \
 	safe_basename(__FILE__, __LINE__, NULL, (path))
 
+#define SAFE_CHROOT(path) \
+	safe_chroot(__FILE__, __LINE__, NULL, (path))
+
 #define SAFE_CHDIR(path) \
 	safe_chdir(__FILE__, __LINE__, NULL, (path))
 
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index c48e436dc..b3c56f47f 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -33,6 +33,21 @@ char *safe_basename(const char *file, const int lineno,
 	return rval;
 }
 
+int safe_chroot(const char *file, const int lineno, void (*cleanup_fn) (void),
+               const char *path)
+{
+	int rval;
+
+	rval = chroot(path);
+	if (rval == -1) {
+		tst_brkm(TBROK | TERRNO, cleanup_fn,
+			 "%s:%d: chroot(%s) failed",
+			 file, lineno, path);
+	}
+
+	return rval;
+}
+
 int
 safe_chdir(const char *file, const int lineno, void (*cleanup_fn) (void),
 	   const char *path)
-- 
2.15.1

[LTP] [RFC PATCH 1/2] lib: Add SAFE_CHROOT(path) macro

[Cyril Hrubis]

Hi!
> diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h
> index 3df952811..9b11801a4 100644
> --- a/include/safe_macros_fn.h
> +++ b/include/safe_macros_fn.h
> @@ -30,6 +30,9 @@ char* safe_basename(const char *file, const int lineno,
>  int safe_chdir(const char *file, const int lineno,
>                 void (*cleanup_fn)(void), const char *path);
>  
> +int safe_chroot(const char *file, const int lineno,
> +               void (*cleanup_fn)(void), const char *path);

Can we please add the safe macro only for the newlib? I.e. function
prototype into the tst_safe_macros.h and implementation into
tst_safe_macros.c?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC PATCH 2/2] cve/cve-2018-1000001: Add Realpath Buffer Underflow test

[Petr Vorel]

Idea based on test from glibc , contributed by Dmitry V. Levin:
52a713fdd0 ("linux: make getcwd(3) fail if it cannot obtain an absolute
path [BZ #22679]")

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: I didn't use TEST() macro due warning assignment makes integer
from pointer without a cast. Am I blind not to see how to use it?
---
 testcases/cve/cve-2018-1000001.c | 66 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 testcases/cve/cve-2018-1000001.c

diff --git a/testcases/cve/cve-2018-1000001.c b/testcases/cve/cve-2018-1000001.c
new file mode 100644
index 000000000..ae41c786f
--- /dev/null
+++ b/testcases/cve/cve-2018-1000001.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2018 Petr Vorel <pvorel@suse.cz>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "tst_test.h"
+
+#include <errno.h>
+#include <stdlib.h>
+
+#define CHROOT_DIR "cve-2018-1000001"
+
+static void setup(void)
+{
+	SAFE_MKDIR(CHROOT_DIR, 0755);
+	SAFE_CHROOT(CHROOT_DIR);
+}
+
+static void run(unsigned int i)
+{
+	char *cwd;
+
+	int fail = 0;
+
+	errno = 0;
+	if (!i) {
+		tst_res(TINFO, "testing getcwd()");
+		cwd = getcwd(NULL, 0);
+	} else {
+		tst_res(TINFO, "testing realpath()");
+		cwd = realpath(".", NULL);
+	}
+
+	if (errno != ENOENT) {
+		tst_res(TFAIL | TERRNO, "returned unexpected errno");
+		fail = 1;
+	}
+
+	if (cwd != NULL) {
+		tst_res(TFAIL, "getcwd() not returned NULL path: '%s'", cwd);
+		fail = 1;
+	}
+
+	if (!fail)
+		tst_res(TPASS, "bug not reproduced");
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = 2,
+	.setup = setup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+};
-- 
2.15.1

[LTP] [RFC PATCH 2/2] cve/cve-2018-1000001: Add Realpath Buffer Underflow test

[Cyril Hrubis]

Hi!
> ---
> NOTE: I didn't use TEST() macro due warning assignment makes integer
> from pointer without a cast. Am I blind not to see how to use it?

You are not, the TEST() macro supports only integer return values.

We may as well add a support for this, maybe just rename the TEST_RETURN
to tst_ret and add void* tst_ret_ptr. If we make the tst_ret to intptr_t
we may as well safely do something as:

	tst_ret_ptr = (void*)(tst_ret = (intptr_t) SCALL);

And we should rename TEST_ERRNO tst_errno as well just to keep it
consistent.

Or we can as well avoid this trickery by defining second TESTPTR() macro
that will use tst_ret_ptr instead.

> ---
>  testcases/cve/cve-2018-1000001.c | 66 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 testcases/cve/cve-2018-1000001.c
> 
> diff --git a/testcases/cve/cve-2018-1000001.c b/testcases/cve/cve-2018-1000001.c
> new file mode 100644
> index 000000000..ae41c786f
> --- /dev/null
> +++ b/testcases/cve/cve-2018-1000001.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (C) 2018 Petr Vorel <pvorel@suse.cz>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "tst_test.h"
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +
> +#define CHROOT_DIR "cve-2018-1000001"
> +
> +static void setup(void)
> +{
> +	SAFE_MKDIR(CHROOT_DIR, 0755);
> +	SAFE_CHROOT(CHROOT_DIR);
> +}
> +
> +static void run(unsigned int i)
> +{
> +	char *cwd;
> +
> +	int fail = 0;
> +
> +	errno = 0;
> +	if (!i) {
> +		tst_res(TINFO, "testing getcwd()");
> +		cwd = getcwd(NULL, 0);
> +	} else {
> +		tst_res(TINFO, "testing realpath()");
> +		cwd = realpath(".", NULL);
> +	}
> +
> +	if (errno != ENOENT) {
> +		tst_res(TFAIL | TERRNO, "returned unexpected errno");
> +		fail = 1;
> +	}
> +
> +	if (cwd != NULL) {
        ^
	No need for the NULL comparsion, can write just:

	if (cwd) {
> +		tst_res(TFAIL, "getcwd() not returned NULL path: '%s'", cwd);
                                ^
				getcwd()/realpath()
> +		fail = 1;
> +	}
> +
> +	if (!fail)
> +		tst_res(TPASS, "bug not reproduced");
> +}
> +
> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = 2,
> +	.setup = setup,
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +};

Other than the very minor nits this looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC PATCH 2/2] cve/cve-2018-1000001: Add Realpath Buffer Underflow test

[Petr Vorel]

Hi Cyril,

thanks for your review and explanation.

> > ---
> > NOTE: I didn't use TEST() macro due warning assignment makes integer
> > from pointer without a cast. Am I blind not to see how to use it?

> You are not, the TEST() macro supports only integer return values.

> We may as well add a support for this, maybe just rename the TEST_RETURN
> to tst_ret and add void* tst_ret_ptr. If we make the tst_ret to intptr_t
> we may as well safely do something as:

> 	tst_ret_ptr = (void*)(tst_ret = (intptr_t) SCALL);

> And we should rename TEST_ERRNO tst_errno as well just to keep it
> consistent.

> Or we can as well avoid this trickery by defining second TESTPTR() macro
> that will use tst_ret_ptr instead.
IMHO this is better.


Kind regards,
Petr