From fff83e7eacd0f27bb2d71c42488e0fd735c15ac3 Mon Sep 17 00:00:00 2001
From: Olaf Meeuwissen <paddy-hack@member.fsf.org>
Date: Thu, 30 Apr 2020 18:24:51 +0900
Subject: [PATCH] epson2: Rewrite network I/O

This addresses GHSL-2020-075 as well as all other problematic code
uncovered as a result of investigating that.  This includes:

- buffer overflows due to use of unchecked lengths
- integer overflows due to type conversions
- potential memory leaks
- checking for memory allocation failures

Re #279.
---
 backend/epson2_net.c | 140 +++++++++++++++++++++++++------------------
 backend/epson2_net.h |   4 +-
 2 files changed, 85 insertions(+), 59 deletions(-)

diff --git a/backend/epson2_net.c b/backend/epson2_net.c
index 8d0fe9ea7..7f804eea8 100644
--- a/backend/epson2_net.c
+++ b/backend/epson2_net.c
@@ -32,11 +32,12 @@
 
 #include "sane/sanei_debug.h"
 
-static int
+static ssize_t
 sanei_epson_net_read_raw(Epson_Scanner *s, unsigned char *buf, ssize_t wanted,
 		       SANE_Status *status)
 {
-	int ready, read = -1;
+	int ready;
+	ssize_t read = -1;
 	fd_set readable;
 	struct timeval tv;
 
@@ -62,111 +63,136 @@ sanei_epson_net_read_raw(Epson_Scanner *s, unsigned char *buf, ssize_t wanted,
 	return read;
 }
 
-int
-sanei_epson_net_read(Epson_Scanner *s, unsigned char *buf, ssize_t wanted,
+static ssize_t
+sanei_epson_net_read_buf(Epson_Scanner *s, unsigned char *buf, ssize_t wanted,
 		       SANE_Status * status)
 {
-	ssize_t size;
 	ssize_t read = 0;
-	unsigned char header[12];
 
-	/* read from buffer, if available */
-	if (s->netptr != s->netbuf) {
-		DBG(23, "reading %lu from buffer at %p, %lu available\n",
-			(u_long) wanted, s->netptr, (u_long) s->netlen);
+	DBG(23, "%s: reading up to %lu from buffer at %p, %lu available\n",
+		__func__, (u_long) wanted, s->netptr, (u_long) s->netlen);
 
-		memcpy(buf, s->netptr, wanted);
-		read = wanted;
+	if ((size_t) wanted > s->netlen) {
+		*status = SANE_STATUS_IO_ERROR;
+		wanted = s->netlen;
+	}
 
-		s->netlen -= wanted;
+	memcpy(buf, s->netptr, wanted);
+	read = wanted;
 
-		if (s->netlen == 0) {
-			DBG(23, "%s: freeing %p\n", __func__, s->netbuf);
-			free(s->netbuf);
-			s->netbuf = s->netptr = NULL;
-			s->netlen = 0;
-		}
+	s->netptr += read;
+	s->netlen -= read;
+
+	if (s->netlen == 0) {
+		DBG(23, "%s: freeing %p\n", __func__, s->netbuf);
+		free(s->netbuf);
+		s->netbuf = s->netptr = NULL;
+		s->netlen = 0;
+	}
+
+	return read;
+}
+
+ssize_t
+sanei_epson_net_read(Epson_Scanner *s, unsigned char *buf, ssize_t wanted,
+		       SANE_Status * status)
+{
+	if (wanted < 0) {
+		*status = SANE_STATUS_INVAL;
+		return 0;
+	}
+
+	size_t size;
+	ssize_t read = 0;
+	unsigned char header[12];
 
-		return read;
+	/* read from remainder of buffer */
+	if (s->netptr) {
+		return sanei_epson_net_read_buf(s, buf, wanted, status);
 	}
 
 	/* receive net header */
-	size = sanei_epson_net_read_raw(s, header, 12, status);
-	if (size != 12) {
+	read = sanei_epson_net_read_raw(s, header, 12, status);
+	if (read != 12) {
 		return 0;
 	}
 
+	/* validate header */
 	if (header[0] != 'I' || header[1] != 'S') {
 		DBG(1, "header mismatch: %02X %02x\n", header[0], header[1]);
 		*status = SANE_STATUS_IO_ERROR;
 		return 0;
 	}
 
+	/* parse payload size */
 	size = be32atoh(&header[6]);
 
-	DBG(23, "%s: wanted = %lu, available = %lu\n", __func__,
-		(u_long) wanted, (u_long) size);
-
 	*status = SANE_STATUS_GOOD;
 
-	if (size == wanted) {
-
-		DBG(15, "%s: full read\n", __func__);
-
-		read = sanei_epson_net_read_raw(s, buf, size, status);
+	if (!s->netbuf) {
+		DBG(15, "%s: direct read\n", __func__);
+		DBG(23, "%s: wanted = %lu, available = %lu\n", __func__,
+			(u_long) wanted, (u_long) size);
 
-		if (s->netbuf) {
-			free(s->netbuf);
-			s->netbuf = NULL;
-			s->netlen = 0;
+		if ((size_t) wanted > size) {
+			wanted = size;
 		}
 
-		if (read < 0) {
-			return 0;
-		}
-
-/*	} else if (wanted < size && s->netlen == size) { */
+		read = sanei_epson_net_read_raw(s, buf, wanted, status);
 	} else {
-		DBG(23, "%s: partial read\n", __func__);
+		DBG(15, "%s: buffered read\n", __func__);
+		DBG(23, "%s: bufferable = %lu, available = %lu\n", __func__,
+			(u_long) s->netlen, (u_long) size);
 
-		read = sanei_epson_net_read_raw(s, s->netbuf, size, status);
-		if (read != size) {
-			return 0;
+		if (s->netlen > size) {
+			s->netlen = size;
 		}
 
-		s->netlen = size - wanted;
-		s->netptr += wanted;
-		read = wanted;
-
-		DBG(23, "0,4 %02x %02x\n", s->netbuf[0], s->netbuf[4]);
-		DBG(23, "storing %lu to buffer at %p, next read at %p, %lu bytes left\n",
-			(u_long) size, s->netbuf, s->netptr, (u_long) s->netlen);
+		/* fill buffer */
+		read = sanei_epson_net_read_raw(s, s->netbuf, s->netlen, status);
+		s->netptr = s->netbuf;
+		s->netlen = (read > 0 ? read : 0);
 
-		memcpy(buf, s->netbuf, wanted);
+		/* copy wanted part */
+		read = sanei_epson_net_read_buf(s, buf, wanted, status);
 	}
 
 	return read;
 }
 
-
-int
+size_t
 sanei_epson_net_write(Epson_Scanner *s, unsigned int cmd, const unsigned char *buf,
 			size_t buf_size, size_t reply_len, SANE_Status *status)
 {
 	unsigned char *h1, *h2, *payload;
 	unsigned char *packet = malloc(12 + 8 + buf_size);
 
-	/* XXX check allocation failure */
+	if (!packet) {
+		*status = SANE_STATUS_NO_MEM;
+		return 0;
+	}
 
 	h1 = packet;
 	h2 = packet + 12;
 	payload = packet + 12 + 8;
 
 	if (reply_len) {
-		s->netbuf = s->netptr = malloc(reply_len);
+		if (s->netbuf) {
+			DBG(23, "%s, freeing %p, %ld bytes unprocessed\n",
+				__func__, s->netbuf, (u_long) s->netlen);
+			free(s->netbuf);
+			s->netbuf = s->netptr = NULL;
+			s->netlen = 0;
+		}
+		s->netbuf = malloc(reply_len);
+		if (!s->netbuf) {
+			free(packet);
+			*status = SANE_STATUS_NO_MEM;
+			return 0;
+		}
 		s->netlen = reply_len;
-		DBG(24, "allocated %lu bytes at %p\n",
-			(u_long) reply_len, s->netbuf);
+		DBG(24, "%s: allocated %lu bytes at %p\n", __func__,
+			(u_long) s->netlen, s->netbuf);
 	}
 
 	DBG(24, "%s: cmd = %04x, buf = %p, buf_size = %lu, reply_len = %lu\n",
diff --git a/backend/epson2_net.h b/backend/epson2_net.h
index 6aef2b725..7db671bf1 100644
--- a/backend/epson2_net.h
+++ b/backend/epson2_net.h
@@ -4,9 +4,9 @@
 #include <sys/types.h>
 #include "../include/sane/sane.h"
 
-extern int sanei_epson_net_read(struct Epson_Scanner *s, unsigned char *buf, ssize_t buf_size,
+extern ssize_t sanei_epson_net_read(struct Epson_Scanner *s, unsigned char *buf, ssize_t buf_size,
 				SANE_Status *status);
-extern int sanei_epson_net_write(struct Epson_Scanner *s, unsigned int cmd, const unsigned char *buf,
+extern size_t sanei_epson_net_write(struct Epson_Scanner *s, unsigned int cmd, const unsigned char *buf,
 				size_t buf_size, size_t reply_len,
 				SANE_Status *status);
 extern SANE_Status sanei_epson_net_lock(struct Epson_Scanner *s);
-- 
GitLab

