[PATCH] libmount: Fix access to uninitialised value in mnt_optstr_locate_option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Sören Tempel <soeren@xxxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxxxx>
---
 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);



[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux