[PATCH 2/4] isrv_identd: Fix off-by-one buffer overflow

Ryan Mallon rmallon at gmail.com
Thu Jan 2 22:13:46 UTC 2014


The buf->buf buffer in do_rd() is explicitly NUL terminated. If the
total number of bytes read is equal to the buffer size, then the NUL
terminator will be written at the byte after the end of the buffer.
Fix this by reducing the size of the read by 1 byte.

Also removed the check for buf->pos being less than
(int)sizeof(buf->buf). This is unnecessary since buf->pos cannot
exceed sizeof(buf->buf), and was also incorrect since it was checked
after buf->buf[buf->pos] was assigned with a NUL value. Removing it
fixes a spurious warning from Smatch (http://smatch.sourceforge.net/).

Signed-off-by: Ryan Mallon <rmallon at gmail.com>
---
 networking/isrv_identd.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/networking/isrv_identd.c b/networking/isrv_identd.c
index d571eb4..e0935c4 100644
--- a/networking/isrv_identd.c
+++ b/networking/isrv_identd.c
@@ -57,8 +57,9 @@ static int do_rd(int fd, void **paramp)
 
 	if (buf->fd_flag & O_NONBLOCK)
 		fcntl(fd, F_SETFL, buf->fd_flag);
-	sz = safe_read(fd, cur, sizeof(buf->buf) - buf->pos);
 
+	/* Leave space for the NUL terminator */
+	sz = safe_read(fd, cur, sizeof(buf->buf) - 1 - buf->pos);
 	if (sz < 0) {
 		if (errno != EAGAIN)
 			goto term; /* terminate this session if !EAGAIN */
@@ -70,7 +71,7 @@ static int do_rd(int fd, void **paramp)
 	p = strpbrk(cur, "\r\n");
 	if (p)
 		*p = '\0';
-	if (!p && sz && buf->pos <= (int)sizeof(buf->buf))
+	if (!p && sz)
 		goto ok;
 	/* Terminate session. If we are in server mode, then
 	 * fd is still in nonblocking mode - we never block here */
-- 
1.7.1



More information about the busybox mailing list