/dev/urandom uses uninit bytes, leaks user data

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Matt Mackall <mpm@...>
Cc: <linux-kernel@...>, <security@...>
Date: Friday, December 14, 2007 - 3:34 pm

xfer_secondary_pool() in drivers/char/random.c tells add_entropy_words()
to use uninitialized tmp[] whenever bytes is not a multiple of 4.
Besides being unfriendly to automated dynamic checkers, this is a
potential leak of user data into the output stream.  When called from
extract_entropy_user, then uninit tmp[] can capture leftover data
from a previous copy_from_user().

Signed off by: jreiser@BitWagon.com

--- ./drivers/char/random.c.orig	2007-12-14 11:06:03.000000000 -0800
+++ ./drivers/char/random.c	2007-12-14 11:06:57.000000000 -0800
@@ -708,7 +708,19 @@

 		bytes=extract_entropy(r->pull, tmp, bytes,
 				      random_read_wakeup_thresh / 8, rsvd);
- 		add_entropy_words(r, tmp, (bytes + 3) / 4);
+		/*
+		 * 2007-12-13 (valgrind/memcheck) Do not use undefined bytes.
+		 * Avoid info leak when called from extract_entropy_user:
+		 * uninit tmp[] can have data from previous copy_from_user().
+		 * Instead: fill last word using first bytes.
+		 */
+		{
+			__u8 *src = (__u8 *)&tmp[0];
+			__u8 *dst = bytes + src;
+			for (; 0!=(3 & bytes); ++bytes)
+				*dst++ = *src++;
+		}
+		add_entropy_words(r, tmp, bytes>>2);
 		credit_entropy_store(r, bytes*8);
 	}

-- 
John Reiser, jreiser@BitWagon.com

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
/dev/urandom uses uninit bytes, leaks user data, John Reiser, (Fri Dec 14, 3:34 pm)