SEO

Hi,

there is a possible (but veeery unlikely) integer overflow in gzsig
during the load of public and private keys. First off I will post the
affected code:

struct stat st;
...
if (fstat(fd, &st) < 0)
return (-1);
...
if (st.st_size == 0) {
errno = EINVAL;
return (-1);
}
if ((iov->iov_base = malloc(st.st_size + 1)) == NULL)
return (-1);

iov->iov_len = st.st_size;
((u_char *)iov->iov_base)[iov->iov_len] = '\0';

if (read(fd, iov->iov_base, iov->iov_len) != iov->iov_len) {

st_size is of type off_t which is 64 bit. malloc expects a variable
of type size_t which is (at least on my arch) 32 bit. If st_size
would be SIZE_T_MAX, malloc would allocate 0 bytes. In this case,
the following steps would assume that the reserved memory block
would be SIZE_T_MAX (because iov_len, which is of type size_t, can
store that value). In this special case it is likely that a
segmentation fault happens as there would be a lot of data in the
file referenced with fd.

Therefore I think st.st_size should be checked for a max value.
My patch suggests ULONG_MAX, perhaps there is a better way for this?

--- usr.bin/gzsig/key.c~ Sat Apr 1 16:46:42 2006
+++ usr.bin/gzsig/key.c Sat Apr 1 16:47:58 2006
@@ -33,6 +33,7 @@
* $Vendor: key.c,v 1.2 2005/04/01 16:47:31 dugsong Exp $
*/

+#include <sys/limits.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/uio.h>
@@ -78,7 +79,7 @@
if (fstat(fd, &st) < 0)
return (-1);

- if (st.st_size == 0) {
+ if (st.st_size == 0 || st.st_size > ULONG_MAX - 1) {
errno = EINVAL;
return (-1);
}


I would like to get feedback (of any kind) about this as I am very
new about these kind of errors. Perhaps my assumption that someone
would craft a public oder private key 8 gb in size is so idiotic that
nobody will care about this ...


Tobias Stoeckmann