From: soeren@soeren-tempel.net
To: util-linux@vger.kernel.org
Subject: [PATCH] libmount: Fix access to uninitialised value in mnt_optstr_locate_option
Date: Sat, 25 Feb 2023 12:43:52 +0100 [thread overview]
Message-ID: <20230225114352.9151-1-soeren@soeren-tempel.net> (raw)
From: Sören Tempel <soeren@soeren-tempel.net>
Consider the following libmount example program:
#include <libmount.h>
int
main(void)
{
mnt_match_options("", "+");
return 0;
}
Compiling this program and executing it with valgrind(1) will yield
the following warning regarding a conditional jump depending on an
uninitialised value:
Conditional jump or move depends on uninitialised value(s)
at 0x48AA61B: strlen (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x48C6154: ??? (in /lib/libmount.so.1.1.0)
by 0x48C65A0: mnt_optstr_get_option (in /lib/libmount.so.1.1.0)
by 0x48C7B85: mnt_match_options (in /lib/libmount.so.1.1.0)
by 0x1091C1: main (util-linux-test.c:6)
This is because if name == "+" then we advance to the null byte
in name due to the following code in mnt_match_options():
if (*name == '+')
name++, namesz--;
This will cause the `xstrncpy(buf, name, namesz + 1)` invocation in
mnt_match_options() to copy nothing to the destination buffer. The
buffer (buf) is therefore passed uninitialized as the name argument
to mnt_optstr_get_option(). When mnt_optstr_locate_option() (which
is called by mnt_optstr_get_option) attempts to determine the
length of the name argument using strlen(3) then everything blows
up because the name argument is not initialized.
This patch fixes this issue by initializing the buf argument in
mnt_match_options() with NULL before calling xstrncpy thereby
ensuring that buf is /always/ initialized even if xstrncpy
returns without copying any data to the destination buffer
due to the following early return in xstrncpy:
size_t len = src ? strlen(src) : 0;
if (!len)
return;
This issue has been discovered using KLEE <https://klee.github.io/>.
Signed-off-by: Sören Tempel <soeren@soeren-tempel.net>
---
libmount/src/optstr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libmount/src/optstr.c b/libmount/src/optstr.c
index a8b56e212..ae3efc78b 100644
--- a/libmount/src/optstr.c
+++ b/libmount/src/optstr.c
@@ -853,6 +853,7 @@ int mnt_match_options(const char *optstr, const char *pattern)
else if ((no = (startswith(name, "no") != NULL)))
name += 2, namesz -= 2;
+ buf = NULL; /* ensure buf is initialized even if name == "" */
xstrncpy(buf, name, namesz + 1);
rc = mnt_optstr_get_option(optstr, buf, &val, &sz);
next reply other threads:[~2023-02-25 11:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-25 11:43 soeren [this message]
2023-02-25 12:41 ` [PATCH v2] libmount: Fix access to uninitialised value in mnt_optstr_locate_option soeren
2023-02-25 13:40 ` Sören Tempel
2023-02-27 10:50 ` Karel Zak
2023-02-27 19:00 ` Sören Tempel
2023-02-28 11:02 ` Karel Zak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230225114352.9151-1-soeren@soeren-tempel.net \
--to=soeren@soeren-tempel.net \
--cc=util-linux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).