Increasing strictness regarding signed-vs-unsigned types and their range

in the io-path, whic is done by seperating int vs uint functions variants.
reviewed by naddy, florian, and jsg

5 files changed, 104 insertions, 67 deletions

diff --git a/usr.bin/rsync/extern.h b/usr.bin/rsync/extern.h
index 86eea421236..12aceae566c 100644
--- a/usr.bin/rsync/extern.h
+++ b/usr.bin/rsync/extern.h
@@ -1,4 +1,4 @@
-/*	$Id: extern.h,v 1.25 2019/03/31 08:47:46 naddy Exp $ */
+/*	$Id: extern.h,v 1.26 2019/03/31 09:26:05 deraadt Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -273,14 +273,17 @@ int		  io_read_byte(struct sess *, int, uint8_t *);
 int		  io_read_check(struct sess *, int);
 int		  io_read_flush(struct sess *, int);
 int		  io_read_int(struct sess *, int, int32_t *);
+int		  io_read_uint(struct sess *, int, uint32_t *);
 int		  io_read_long(struct sess *, int, int64_t *);
 int		  io_read_size(struct sess *, int, size_t *);
 int		  io_read_ulong(struct sess *, int, uint64_t *);
 int		  io_write_buf(struct sess *, int, const void *, size_t);
 int		  io_write_byte(struct sess *, int, uint8_t);
 int		  io_write_int(struct sess *, int, int32_t);
+int		  io_write_uint(struct sess *, int, uint32_t);
 int		  io_write_line(struct sess *, int, const char *);
 int		  io_write_long(struct sess *, int, int64_t);
+int		  io_write_ulong(struct sess *, int, uint64_t);
 
 int		  io_lowbuffer_alloc(struct sess *, void **,
 			size_t *, size_t *, size_t);
diff --git a/usr.bin/rsync/flist.c b/usr.bin/rsync/flist.c
index 0927bd8841d..b39599583e3 100644
--- a/usr.bin/rsync/flist.c
+++ b/usr.bin/rsync/flist.c
@@ -1,4 +1,4 @@
-/*	$Id: flist.c,v 1.22 2019/03/23 23:22:57 deraadt Exp $ */
+/*	$Id: flist.c,v 1.23 2019/03/31 09:26:05 deraadt Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2019 Florian Obser <florian@openbsd.org>
@@ -272,6 +272,7 @@ flist_send(struct sess *sess, int fdin, int fdout, const struct flist *fl,
 		fn = f->wpath;
 		sz = strlen(f->wpath);
 		assert(sz > 0);
+		assert(sz < INT32_MAX);
 
 		/*
 		 * If applicable, unclog the read buffer.
@@ -318,19 +319,19 @@ flist_send(struct sess *sess, int fdin, int fdout, const struct flist *fl,
 		} else if (!io_write_long(sess, fdout, f->st.size)) {
 			ERRX1(sess, "io_write_long");
 			goto out;
-		} else if (!io_write_int(sess, fdout, f->st.mtime)) {
-			ERRX1(sess, "io_write_int");
+		} else if (!io_write_uint(sess, fdout, (uint32_t)f->st.mtime)) {
+			ERRX1(sess, "io_write_uint");
 			goto out;
-		} else if (!io_write_int(sess, fdout, f->st.mode)) {
-			ERRX1(sess, "io_write_int");
+		} else if (!io_write_uint(sess, fdout, f->st.mode)) {
+			ERRX1(sess, "io_write_uint");
 			goto out;
 		}
 
 		/* Conditional part: uid. */
 
 		if (sess->opts->preserve_uids) {
-			if (!io_write_int(sess, fdout, f->st.uid)) {
-				ERRX1(sess, "io_write_int");
+			if (!io_write_uint(sess, fdout, f->st.uid)) {
+				ERRX1(sess, "io_write_uint");
 				goto out;
 			}
 			if (!idents_add(sess, 0, &uids, &uidsz, f->st.uid)) {
@@ -342,8 +343,8 @@ flist_send(struct sess *sess, int fdin, int fdout, const struct flist *fl,
 		/* Conditional part: gid. */
 
 		if (sess->opts->preserve_gids) {
-			if (!io_write_int(sess, fdout, f->st.gid)) {
-				ERRX1(sess, "io_write_int");
+			if (!io_write_uint(sess, fdout, f->st.gid)) {
+				ERRX1(sess, "io_write_uint");
 				goto out;
 			}
 			if (!idents_add(sess, 1, &gids, &gidsz, f->st.gid)) {
@@ -370,12 +371,13 @@ flist_send(struct sess *sess, int fdin, int fdout, const struct flist *fl,
 		    sess->opts->preserve_links) {
 			fn = f->link;
 			sz = strlen(f->link);
+			assert(sz < INT32_MAX);
 			if (!io_write_int(sess, fdout, sz)) {
 				ERRX1(sess, "io_write_int");
 				goto out;
 			}
 			if (!io_write_buf(sess, fdout, fn, sz)) {
-				ERRX1(sess, "io_write_int");
+				ERRX1(sess, "io_write_buf");
 				goto out;
 			}
 		}
@@ -593,7 +595,7 @@ flist_recv(struct sess *sess, int fd, struct flist **flp, size_t *sz)
 	size_t		 flsz = 0, flmax = 0, lsz, gidsz = 0, uidsz = 0;
 	uint8_t		 flag;
 	char		 last[MAXPATHLEN];
-	uint64_t	 lval; /* temporary values... */
+	int64_t		 lval; /* temporary values... */
 	int32_t		 ival;
 	uint32_t	 uival;
 	struct ident	*gids = NULL, *uids = NULL;
@@ -624,8 +626,8 @@ flist_recv(struct sess *sess, int fd, struct flist **flp, size_t *sz)
 
 		/* Read the file size. */
 
-		if (!io_read_ulong(sess, fd, &lval)) {
-			ERRX1(sess, "io_read_ulong");
+		if (!io_read_long(sess, fd, &lval)) {
+			ERRX1(sess, "io_read_long");
 			goto out;
 		}
 		ff->st.size = lval;
@@ -633,7 +635,7 @@ flist_recv(struct sess *sess, int fd, struct flist **flp, size_t *sz)
 		/* Read the modification time. */
 
 		if (!(FLIST_TIME_SAME & flag)) {
-			if (!io_read_int(sess, fd, &uival)) {
+			if (!io_read_uint(sess, fd, &uival)) {
 				ERRX1(sess, "io_read_int");
 				goto out;
 			}
@@ -647,11 +649,11 @@ flist_recv(struct sess *sess, int fd, struct flist **flp, size_t *sz)
 		/* Read the file mode. */
 
 		if (!(FLIST_MODE_SAME & flag)) {
-			if (!io_read_int(sess, fd, &ival)) {
+			if (!io_read_uint(sess, fd, &uival)) {
 				ERRX1(sess, "io_read_int");
 				goto out;
 			}
-			ff->st.mode = ival;
+			ff->st.mode = uival;
 		} else if (fflast == NULL) {
 			ERRX(sess, "same mode without last entry");
 			goto out;
@@ -662,11 +664,11 @@ flist_recv(struct sess *sess, int fd, struct flist **flp, size_t *sz)
 
 		if (sess->opts->preserve_uids) {
 			if (!(FLIST_UID_SAME & flag)) {
-				if (!io_read_int(sess, fd, &ival)) {
+				if (!io_read_uint(sess, fd, &uival)) {
 					ERRX1(sess, "io_read_int");
 					goto out;
 				}
-				ff->st.uid = ival;
+				ff->st.uid = uival;
 			} else if (fflast == NULL) {
 				ERRX(sess, "same uid without last entry");
 				goto out;
@@ -678,11 +680,11 @@ flist_recv(struct sess *sess, int fd, struct flist **flp, size_t *sz)
 
 		if (sess->opts->preserve_gids) {
 			if (!(FLIST_GID_SAME & flag)) {
-				if (!io_read_int(sess, fd, &ival)) {
+				if (!io_read_uint(sess, fd, &uival)) {
 					ERRX1(sess, "io_read_int");
 					goto out;
 				}
-				ff->st.gid = ival;
+				ff->st.gid = uival;
 			} else if (fflast == NULL) {
 				ERRX(sess, "same gid without last entry");
 				goto out;
diff --git a/usr.bin/rsync/ids.c b/usr.bin/rsync/ids.c
index 3b0367883a8..b946dd33ceb 100644
--- a/usr.bin/rsync/ids.c
+++ b/usr.bin/rsync/ids.c
@@ -1,4 +1,4 @@
-/*	$Id: ids.c,v 1.10 2019/03/30 07:24:02 deraadt Exp $ */
+/*	$Id: ids.c,v 1.11 2019/03/31 09:26:05 deraadt Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -103,7 +103,8 @@ idents_remap(struct sess *sess, int isgid, struct ident *ids, size_t idsz)
 	size_t		 i;
 	struct group	*grp;
 	struct passwd	*usr;
-	int32_t		 id;
+	uint32_t	 id;
+	int		valid;
 
 	assert(!sess->opts->numeric_ids);
 
@@ -112,12 +113,20 @@ idents_remap(struct sess *sess, int isgid, struct ident *ids, size_t idsz)
 
 		/* Start by getting our local representation. */
 
-		if (isgid)
-			id = (grp = getgrnam(ids[i].name)) == NULL ?
-				-1 : grp->gr_gid;
-		else
-			id = (usr = getpwnam(ids[i].name)) == NULL ?
-				-1 : usr->pw_uid;
+		valid = id = 0;
+		if (isgid) {
+			grp = getgrnam(ids[i].name);
+			if (grp) {
+				id = grp->gr_gid;
+				valid = 1;
+			}
+		} else {
+			usr = getpwnam(ids[i].name);
+			if (usr) {
+				id = usr->pw_uid;
+				valid = 1;
+			}
+		}
 
 		/*
 		 * (1) Empty names inherit.
@@ -128,7 +137,7 @@ idents_remap(struct sess *sess, int isgid, struct ident *ids, size_t idsz)
 
 		if (ids[i].name[0] == '\0')
 			ids[i].mapped = ids[i].id;
-		else if (id <= 0)
+		else if (!valid)
 			ids[i].mapped = ids[i].id;
 		else
 			ids[i].mapped = id;
@@ -229,8 +238,8 @@ idents_send(struct sess *sess,
 		assert(ids[i].id != 0);
 		sz = strlen(ids[i].name);
 		assert(sz > 0 && sz <= UINT8_MAX);
-		if (!io_write_int(sess, fd, ids[i].id)) {
-			ERRX1(sess, "io_write_int");
+		if (!io_write_uint(sess, fd, ids[i].id)) {
+			ERRX1(sess, "io_write_uint");
 			return 0;
 		} else if (!io_write_byte(sess, fd, sz)) {
 			ERRX1(sess, "io_write_byte");
@@ -264,8 +273,8 @@ idents_recv(struct sess *sess,
 	void	*pp;
 
 	for (;;) {
-		if (!io_read_int(sess, fd, &id)) {
-			ERRX1(sess, "io_read_int");
+		if (!io_read_uint(sess, fd, &id)) {
+			ERRX1(sess, "io_read_uint");
 			return 0;
 		} else if (id == 0)
 			break;
diff --git a/usr.bin/rsync/io.c b/usr.bin/rsync/io.c
index 43659dddd74..0e451226d31 100644
--- a/usr.bin/rsync/io.c
+++ b/usr.bin/rsync/io.c
@@ -1,4 +1,4 @@
-/*	$Id: io.c,v 1.14 2019/03/26 17:13:42 deraadt Exp $ */
+/*	$Id: io.c,v 1.15 2019/03/31 09:26:05 deraadt Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -389,13 +389,14 @@ io_read_buf(struct sess *sess, int fd, void *buf, size_t sz)
  * Returns zero on failure, non-zero on success.
  */
 int
-io_write_long(struct sess *sess, int fd, int64_t val)
+io_write_ulong(struct sess *sess, int fd, uint64_t val)
 {
-	int64_t	nv;
+	uint64_t	nv;
+	int64_t		sval = (int64_t)val;
 
 	/* Short-circuit: send as an integer if possible. */
 
-	if (val <= INT32_MAX && val >= 0) {
+	if (sval <= INT32_MAX && sval >= 0) {
 		if (!io_write_int(sess, fd, (int32_t)val)) {
 			ERRX1(sess, "io_write_int");
 			return 0;
@@ -403,7 +404,7 @@ io_write_long(struct sess *sess, int fd, int64_t val)
 		return 1;
 	}
 
-	/* Otherwise, pad with max integer, then send 64-bit. */
+	/* Otherwise, pad with -1 32-bit, then send 64-bit. */
 
 	nv = htole64(val);
 
@@ -417,18 +418,24 @@ io_write_long(struct sess *sess, int fd, int64_t val)
 	return 0;
 }
 
+int
+io_write_long(struct sess *sess, int fd, int64_t val)
+{
+	return io_write_ulong(sess, fd, (uint64_t)val);
+}
+
 /*
- * Like io_write_buf(), but for an integer.
+ * Like io_write_buf(), but for an unsigned integer.
  * Returns zero on failure, non-zero on success.
  */
 int
-io_write_int(struct sess *sess, int fd, int32_t val)
+io_write_uint(struct sess *sess, int fd, uint32_t val)
 {
-	int32_t	nv;
+	uint32_t	nv;
 
 	nv = htole32(val);
 
-	if (!io_write_buf(sess, fd, &nv, sizeof(int32_t))) {
+	if (!io_write_buf(sess, fd, &nv, sizeof(uint32_t))) {
 		ERRX1(sess, "io_write_buf");
 		return 0;
 	}
@@ -436,6 +443,16 @@ io_write_int(struct sess *sess, int fd, int32_t val)
 }
 
 /*
+ * Like io_write_buf(), but for an integer.
+ * Returns zero on failure, non-zero on success.
+ */
+int
+io_write_int(struct sess *sess, int fd, int32_t val)
+{
+	return io_write_uint(sess, fd, (uint32_t)val);
+}
+
+/*
  * A simple assertion-protected memory copy from th einput "val" or size
  * "valsz" into our buffer "buf", full size "buflen", position "bufpos".
  * Increases our "bufpos" appropriately.
@@ -540,19 +557,19 @@ io_buffer_int(struct sess *sess, void *buf,
  * Returns zero on failure, non-zero on success.
  */
 int
-io_read_ulong(struct sess *sess, int fd, uint64_t *val)
+io_read_long(struct sess *sess, int fd, int64_t *val)
 {
-	int64_t	oval;
+	uint64_t	uoval;
 
-	if (!io_read_long(sess, fd, &oval)) {
+	if (!io_read_ulong(sess, fd, &uoval)) {
 		ERRX1(sess, "io_read_long");
 		return 0;
-	} else if (oval < 0) {
-		ERRX(sess, "io_read_size: negative value");
-		return 1;
 	}
-
-	*val = oval;
+	*val = (int64_t)uoval;
+	if (*val < 0) {
+		ERRX1(sess, "io_read_long negative");
+		return 0;
+	}
 	return 1;
 }
 
@@ -561,10 +578,10 @@ io_read_ulong(struct sess *sess, int fd, uint64_t *val)
  * Returns zero on failure, non-zero on success.
  */
 int
-io_read_long(struct sess *sess, int fd, int64_t *val)
+io_read_ulong(struct sess *sess, int fd, uint64_t *val)
 {
-	int64_t	oval;
-	int32_t sval;
+	uint64_t	 oval;
+	int32_t		 sval;
 
 	/* Start with the short-circuit: read as an int. */
 
@@ -572,13 +589,13 @@ io_read_long(struct sess *sess, int fd, int64_t *val)
 		ERRX1(sess, "io_read_int");
 		return 0;
 	} else if (sval != -1) {
-		*val = sval;
+		*val = (uint64_t)le32toh(sval);
 		return 1;
 	}
 
-	/* If the int is maximal, read as 64 bits. */
+	/* If the int is -1, read as 64 bits. */
 
-	if (!io_read_buf(sess, fd, &oval, sizeof(int64_t))) {
+	if (!io_read_buf(sess, fd, &oval, sizeof(uint64_t))) {
 		ERRX1(sess, "io_read_buf");
 		return 0;
 	}
@@ -616,11 +633,11 @@ io_read_size(struct sess *sess, int fd, size_t *val)
  * Returns zero on failure, non-zero on success.
  */
 int
-io_read_int(struct sess *sess, int fd, int32_t *val)
+io_read_uint(struct sess *sess, int fd, uint32_t *val)
 {
-	int32_t	oval;
+	uint32_t	oval;
 
-	if (!io_read_buf(sess, fd, &oval, sizeof(int32_t))) {
+	if (!io_read_buf(sess, fd, &oval, sizeof(uint32_t))) {
 		ERRX1(sess, "io_read_buf");
 		return 0;
 	}
@@ -629,6 +646,12 @@ io_read_int(struct sess *sess, int fd, int32_t *val)
 	return 1;
 }
 
+int
+io_read_int(struct sess *sess, int fd, int32_t *val)
+{
+	return io_read_uint(sess, fd, (uint32_t *)val);
+}
+
 /*
  * Copies "valsz" from "buf", full size "bufsz" at position" bufpos",
  * into "val".
diff --git a/usr.bin/rsync/session.c b/usr.bin/rsync/session.c
index a72006b07dc..8d71118c011 100644
--- a/usr.bin/rsync/session.c
+++ b/usr.bin/rsync/session.c
@@ -1,4 +1,4 @@
-/*	$Id: session.c,v 1.5 2019/03/23 16:04:28 deraadt Exp $ */
+/*	$Id: session.c,v 1.6 2019/03/31 09:26:05 deraadt Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -111,14 +111,14 @@ sess_stats_send(struct sess *sess, int fd)
 	ts = sess->total_size;
 
 	if (sess->opts->server) {
-		if (!io_write_long(sess, fd, tr)) {
-			ERRX1(sess, "io_write_long");
+		if (!io_write_ulong(sess, fd, tr)) {
+			ERRX1(sess, "io_write_ulong");
 			return 0;
-		} else if (!io_write_long(sess, fd, tw)) {
-			ERRX1(sess, "io_write_long");
+		} else if (!io_write_ulong(sess, fd, tw)) {
+			ERRX1(sess, "io_write_ulong");
 			return 0;
-		} else if (!io_write_long(sess, fd, ts)) {
-			ERRX1(sess, "io_write_long");
+		} else if (!io_write_ulong(sess, fd, ts)) {
+			ERRX1(sess, "io_write_ulong");
 			return 0;
 		}
 	}