dovecot-1.1: random_fill(): If read(/dev/urandom) returned EINTR...

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-1.1/rev/cdb007c1923d
changeset: 7436:cdb007c1923d
user:      Timo Sirainen <tss at iki.fi>
date:      Sun Mar 23 17:55:51 2008 +0200
description:
random_fill(): If read(/dev/urandom) returned EINTR, it could have written
random data before the given buffer (buffer underflow). Pointed out by
Sami Farin.

This function is used only by auth and pop3-login (with APOP enabled)
processes, so normal users shouldn't be able to send signals to any of them to
exploit this. Even then the data would be random, making it quite unlikely to
cause anything else than a crash.

diffstat:

1 file changed, 14 insertions(+), 6 deletions(-)
src/lib/randgen.c |   20 ++++++++++++++------

diffs (49 lines):

diff -r 6983dfc231d7 -r cdb007c1923d src/lib/randgen.c
--- a/src/lib/randgen.c	Fri Mar 21 08:27:36 2008 +0200
+++ b/src/lib/randgen.c	Sun Mar 23 17:55:51 2008 +0200
@@ -6,6 +6,8 @@
 #include <stdlib.h>
 
 #ifdef HAVE_DEV_URANDOM
+
+#define URANDOM_PATH "/dev/urandom"
 
 #include "fd-close-on-exec.h"
 #include <unistd.h>
@@ -22,10 +24,16 @@ void random_fill(void *buf, size_t size)
 	i_assert(init_refcount > 0);
 	i_assert(size < SSIZE_T_MAX);
 
-	for (pos = 0; pos < size; pos += ret) {
+	for (pos = 0; pos < size; ) {
 		ret = read(urandom_fd, (char *) buf + pos, size - pos);
-		if (unlikely(ret < 0 && errno != EINTR))
-			i_fatal("Error reading from /dev/urandom: %m");
+		if (unlikely(ret <= 0)) {
+			if (ret == 0)
+				i_fatal("EOF when reading from "URANDOM_PATH);
+			else if (errno != EINTR)
+				i_fatal("read("URANDOM_PATH") failed: %m");
+		} else {
+			pos += ret;
+		}
 	}
 }
 
@@ -36,13 +44,13 @@ void random_init(void)
 	if (init_refcount++ > 0)
 		return;
 
-	urandom_fd = open("/dev/urandom", O_RDONLY);
+	urandom_fd = open(URANDOM_PATH, O_RDONLY);
 	if (urandom_fd == -1) {
 		if (errno == ENOENT) {
-			i_fatal("/dev/urandom doesn't exist, "
+			i_fatal(URANDOM_PATH" doesn't exist, "
 				"currently we require it");
 		} else {
-			i_fatal("Can't open /dev/urandom: %m");
+			i_fatal("Can't open "URANDOM_PATH": %m");
 		}
 	}