diff mbox series

[v1] mq_notify03.c: New test CVE-2021-38604

Message ID 20230215144820.17876-1-wegao@suse.com
State Changes Requested
Headers show
Series [v1] mq_notify03.c: New test CVE-2021-38604 | expand

Commit Message

Wei Gao Feb. 15, 2023, 2:48 p.m. UTC
This test is come from glibc test mq_notify.c.
Implements following logic:
1) Create POSIX message queue.
   Register a notification with mq_notify (using NULL attributes).
   Then immediately unregister the notification with mq_notify.
   Helper thread in a vulnerable version of glibc
   should cause NULL pointer dereference after these steps.
2) Once again, register the same notification.
   Try to send a dummy message.
   Test is considered successfulif the dummy message
   is successfully received by the callback function.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/cve                                   |   1 +
 runtest/syscalls                              |   1 +
 .../kernel/syscalls/mq_notify/.gitignore      |   1 +
 .../kernel/syscalls/mq_notify/mq_notify03.c   | 105 ++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mq_notify/mq_notify03.c

Comments

Cyril Hrubis Feb. 17, 2023, 4:05 p.m. UTC | #1
Hi!
> +/*\
> + * [Description]
> + *
> + * Test for NULL pointer dereference in mq_notify(CVE-2021-38604)
> + *
> + * References links:
> + * - https://sourceware.org/bugzilla/show_bug.cgi?id=28213
> + */
> +
> +#include <errno.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <mqueue.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include "tst_test.h"
> +#include "tst_safe_posix_ipc.h"
> +
> +static mqd_t m = -1;
> +static const char msg[] = "hello";
> +
> +static void check_bz28213_cb(union sigval sv)
> +{
> +	char buf[sizeof(msg)];
> +
> +	(void)sv;
> +
> +	TST_EXP_PASS(!((size_t) mq_receive(m, buf, sizeof(buf), NULL)

Does this line of code even compile?

> +	TST_EXP_PASS(!(memcmp(buf, msg, sizeof(buf)) == 0));
> +
> +	exit(0);
> +}
> +
> +static void check_bz28213(void)
> +{
> +	struct sigevent sev;
> +
> +	memset(&sev, '\0', sizeof(sev));
> +	sev.sigev_notify = SIGEV_THREAD;
> +	sev.sigev_notify_function = check_bz28213_cb;
> +
> +	/* Step 1: Register & unregister notifier.
> +	 * Helper thread should receive NOTIFY_REMOVED notification.
> +	 * In a vulnerable version of glibc, NULL pointer dereference follows.
> +	 */
> +	TST_EXP_PASS(!(mq_notify(m, &sev) == 0));
> +	TST_EXP_PASS(!(mq_notify(m, NULL) == 0));

That's not how use use the TST_EXP_PASS() macro, the bare mq_notify()
call should be inside.

> +	/* Step 2: Once again, register notification.
> +	 * Try to send one message.
> +	 * Test is considered successful, if the callback does exit (0).
> +	 */
> +	TST_EXP_PASS(!(mq_notify(m, &sev) == 0));
> +	TST_EXP_PASS(!(mq_send(m, msg, sizeof(msg), 1) == 0));

Here as well.

> +	/* Wait... */
> +	pause();
> +}
> +
> +static void do_test(void)
> +{
> +	static const char m_name[] = "/bz28213_queue";
                                       ^
				       We tend to prefix globaly visible
				       object with ltp_ and use the test
				       name in there, so in this case
				       this would be "/ltp_mq_notify03"
> +	struct mq_attr m_attr;
> +
> +	memset(&m_attr, '\0', sizeof(m_attr));
> +	m_attr.mq_maxmsg = 1;
> +	m_attr.mq_msgsize = sizeof(msg);
> +
> +	m = SAFE_MQ_OPEN(m_name,
> +			O_RDWR | O_CREAT | O_EXCL,
> +			0600,
> +			&m_attr);
> +
> +	if (m < 0) {
> +		if (errno == ENOSYS)
> +			tst_brk(TCONF, "POSIX message queues are not implemented");
> +		tst_brk(TFAIL | TTERRNO, "mq_open failed");
> +	}

This will never work, the SAFE_MQ_OPEN() will exit the test if the call
fails with ENOSYS. You have to check for the support in a test setup
instead.

Also I think that unlike the SysV IPC the POSIX IPC cannot be disabled
in kernel .config, so ENOSYS handling may not be needed after all.

> +	TST_EXP_PASS(!(mq_unlink(m_name) == 0));

Here as well.

> +	check_bz28213();
               ^
	       This is poorly choosen name for a function, can we please
	       call this more descriptive name? What about
	       try_null_dereference() ?
> +}
> +
> +
> +static struct tst_test test = {
> +	.test_all = do_test,
> +	.tags = (const struct tst_tag[]) {
> +		{"glibc-git", "b805aebd42"},
> +		{"CVE", "2021-38604"},
> +		{}
> +	},
> +	.needs_root = 1,
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
Wei Gao Feb. 21, 2023, 2:04 a.m. UTC | #2
On Fri, Feb 17, 2023 at 05:05:06PM +0100, Cyril Hrubis wrote:
> Hi!
> > +/*\
> > + * [Description]
> > + *
> > + * Test for NULL pointer dereference in mq_notify(CVE-2021-38604)
> > + *
> > + * References links:
> > + * - https://sourceware.org/bugzilla/show_bug.cgi?id=28213
> > + */
> > +
> > +#include <errno.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <mqueue.h>
> > +#include <signal.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include "tst_test.h"
> > +#include "tst_safe_posix_ipc.h"
> > +
> > +static mqd_t m = -1;
> > +static const char msg[] = "hello";
> > +
> > +static void check_bz28213_cb(union sigval sv)
> > +{
> > +	char buf[sizeof(msg)];
> > +
> > +	(void)sv;
> > +
> > +	TST_EXP_PASS(!((size_t) mq_receive(m, buf, sizeof(buf), NULL)
> 
> Does this line of code even compile?
Yes, this wrong code can pass complie : )
diff mbox series

Patch

diff --git a/runtest/cve b/runtest/cve
index 1ba63c2a7..07bcac0b0 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -74,6 +74,7 @@  cve-2021-26708 vsock01
 cve-2021-22600 setsockopt09
 cve-2022-0847 dirtypipe
 cve-2022-2590 dirtyc0w_shmem
+cve-2021-38604 mq_notify03
 # Tests below may cause kernel memory leak
 cve-2020-25704 perf_event_open03
 cve-2022-4378 cve-2022-4378
diff --git a/runtest/syscalls b/runtest/syscalls
index 81c30402b..536140a3e 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -832,6 +832,7 @@  pkey01 pkey01
 
 mq_notify01 mq_notify01
 mq_notify02 mq_notify02
+mq_notify03 mq_notify03
 mq_open01 mq_open01
 mq_timedreceive01 mq_timedreceive01
 mq_timedsend01 mq_timedsend01
diff --git a/testcases/kernel/syscalls/mq_notify/.gitignore b/testcases/kernel/syscalls/mq_notify/.gitignore
index cca05a7fa..3f9403c05 100644
--- a/testcases/kernel/syscalls/mq_notify/.gitignore
+++ b/testcases/kernel/syscalls/mq_notify/.gitignore
@@ -1,2 +1,3 @@ 
 /mq_notify01
 /mq_notify02
+/mq_notify03
diff --git a/testcases/kernel/syscalls/mq_notify/mq_notify03.c b/testcases/kernel/syscalls/mq_notify/mq_notify03.c
new file mode 100644
index 000000000..6b31e1df1
--- /dev/null
+++ b/testcases/kernel/syscalls/mq_notify/mq_notify03.c
@@ -0,0 +1,105 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) The GNU Toolchain Authors.
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
+ *
+ */
+
+/*\
+ * [Description]
+ *
+ * Test for NULL pointer dereference in mq_notify(CVE-2021-38604)
+ *
+ * References links:
+ * - https://sourceware.org/bugzilla/show_bug.cgi?id=28213
+ */
+
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <mqueue.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include "tst_test.h"
+#include "tst_safe_posix_ipc.h"
+
+static mqd_t m = -1;
+static const char msg[] = "hello";
+
+static void check_bz28213_cb(union sigval sv)
+{
+	char buf[sizeof(msg)];
+
+	(void)sv;
+
+	TST_EXP_PASS(!((size_t) mq_receive(m, buf, sizeof(buf), NULL)
+				== sizeof(buf)));
+	TST_EXP_PASS(!(memcmp(buf, msg, sizeof(buf)) == 0));
+
+	exit(0);
+}
+
+static void check_bz28213(void)
+{
+	struct sigevent sev;
+
+	memset(&sev, '\0', sizeof(sev));
+	sev.sigev_notify = SIGEV_THREAD;
+	sev.sigev_notify_function = check_bz28213_cb;
+
+	/* Step 1: Register & unregister notifier.
+	 * Helper thread should receive NOTIFY_REMOVED notification.
+	 * In a vulnerable version of glibc, NULL pointer dereference follows.
+	 */
+	TST_EXP_PASS(!(mq_notify(m, &sev) == 0));
+	TST_EXP_PASS(!(mq_notify(m, NULL) == 0));
+
+	/* Step 2: Once again, register notification.
+	 * Try to send one message.
+	 * Test is considered successful, if the callback does exit (0).
+	 */
+	TST_EXP_PASS(!(mq_notify(m, &sev) == 0));
+	TST_EXP_PASS(!(mq_send(m, msg, sizeof(msg), 1) == 0));
+
+	/* Wait... */
+	pause();
+}
+
+static void do_test(void)
+{
+	static const char m_name[] = "/bz28213_queue";
+	struct mq_attr m_attr;
+
+	memset(&m_attr, '\0', sizeof(m_attr));
+	m_attr.mq_maxmsg = 1;
+	m_attr.mq_msgsize = sizeof(msg);
+
+	m = SAFE_MQ_OPEN(m_name,
+			O_RDWR | O_CREAT | O_EXCL,
+			0600,
+			&m_attr);
+
+	if (m < 0) {
+		if (errno == ENOSYS)
+			tst_brk(TCONF, "POSIX message queues are not implemented");
+		tst_brk(TFAIL | TTERRNO, "mq_open failed");
+	}
+
+	TST_EXP_PASS(!(mq_unlink(m_name) == 0));
+
+	check_bz28213();
+}
+
+
+static struct tst_test test = {
+	.test_all = do_test,
+	.tags = (const struct tst_tag[]) {
+		{"glibc-git", "b805aebd42"},
+		{"CVE", "2021-38604"},
+		{}
+	},
+	.needs_root = 1,
+};