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.

--HG--
branch : HEAD

diff --git a/src/lib/randgen.c b/src/lib/randgen.c
index 859f6e6ef017997e98bcd4f4b34bbbe903e87303..3521f872413b33301858efaedb1e37a72374c45d 100644 (file)
--- a/src/lib/randgen.c
+++ b/src/lib/randgen.c
@@ -7,6 +7,8 @@
 
 #ifdef HAVE_DEV_URANDOM
 
+#define URANDOM_PATH "/dev/urandom"
+
 #include "fd-close-on-exec.h"
 #include <unistd.h>
 #include <fcntl.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");
                }
        }