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

Cyril Hrubis
2023-02-17T16:03:54Z

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

--
Cyril Hrubis
chrubis@suse.cz