Skip to contentSkip to conversation

CVE-2018-1000001 (glibc)

RFC PATCH New Petr Vorel 2 participants 6 messages Jan 18, 2018 – Jan 19, 2018 Library code

lore
Share & tools
lore

Petr Vorel

author
#0 lore raw

cover CVE-2018-1000001 (glibc)

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
Signature · 1 line
2.15.1
author
#1 lore raw

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

---
 include/safe_macros_fn.h  |  3 +++
 include/tst_safe_macros.h |  5 ++++-
 lib/safe_macros.c         | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)
include/safe_macros_fn.h +3 -0
index 3df952811..9b11801a4 100644
@@ -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);
 
include/tst_safe_macros.h +4 -1
index 06bff13c7..66678dd76 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2015 Linux Test Project
+ * Copyright (c) 2010-2018 Linux Test Project
  * Copyright (c) 2011-2015 Cyril Hrubis <[email protected]>
  *
  * 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))
 
lib/safe_macros.c +15 -0
index c48e436dc..b3c56f47f 100644
@@ -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)
Signature · 1 line
2.15.1
#2 lore raw

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

Hi!

include/safe_macros_fn.h char* safe_basename(const char *file, const int lineno, · line 34

Quoted 10 lines
include/safe_macros_fn.h +2 -0
index 3df952811..9b11801a4 100644
@@ -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?

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#3 lore raw

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

In reply to 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]")

---
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
testcases/cve/cve-2018-1000001.c new file +66 -0
new file mode 100644
index 000000000..ae41c786f
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2018 Petr Vorel <[email protected]>
+ *
+ * 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,
+};
Signature · 1 line
2.15.1
#4 lore raw

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

Hi!

Quoted 3 lines

---
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.

Quoted 52 lines
---
 testcases/cve/cve-2018-1000001.c | 66 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 testcases/cve/cve-2018-1000001.c
testcases/cve/cve-2018-1000001.c new file +41 -0
new file mode 100644
index 000000000..ae41c786f
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2018 Petr Vorel <[email protected]>
+ *
+ * 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 {

testcases/cve/cve-2018-1000001.c line 51

Quoted 10 lines
testcases/cve/cve-2018-1000001.c new file +10 -0
+		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) {

testcases/cve/cve-2018-1000001.c line 52

Quoted 1 line
testcases/cve/cve-2018-1000001.c new file +1 -0
+		tst_res(TFAIL, "getcwd() not returned NULL path: '%s'", cwd);
                                ^
				getcwd()/realpath()
Quoted 4 lines
testcases/cve/cve-2018-1000001.c new file +4 -0
+		fail = 1;
+	}
+
+	if (!fail)

testcases/cve/cve-2018-1000001.c line 66

Quoted 10 lines
testcases/cve/cve-2018-1000001.c new file +10 -0
+		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.

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#5 lore raw

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

Hi Cyril,

thanks for your review and explanation.

Quoted 10 lines
Quoted 3 lines

---
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:

Quoted 7 lines
	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