mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <sha@pengutronix.de>
To: Neeraj Pal <neerajpal09@gmail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [BUG] Stack buffer overflow WRITE of size 1 in nfs_start function
Date: Fri, 7 May 2021 10:41:02 +0200	[thread overview]
Message-ID: <20210507084102.GU19819@pengutronix.de> (raw)
In-Reply-To: <CANi4_RUhZuWvG+v9R1Ae5BmCtHx-Bz_4Ay0BiuBYCrqXUAOBiw@mail.gmail.com>

Hi,

On Sun, Apr 18, 2021 at 12:22:30AM +0530, Neeraj Pal wrote:
> Hi,
> 
> While reviewing the code of barebox-2021.04.0 and git commit
> af0f068a6edad45b033e772056ac0352e1ba3613  I found a stack buffer
> overflow WRITE of size 1 in
> nfs_start() net/nfs.c L664 through strcpy call which furthers crashes at
> function strcpy in lib/string.c L96.

Thanks for reporting this. Indeed the nfs filename is stored in a fixed
size buffer which can easily overflow with the right input.

This patch should fix this issue.

Regards,
  Sascha

-----------------------------8<---------------------------------
>From 3978396bf88c4ab567ddf36dff1218502e32a94d Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Fri, 7 May 2021 10:26:51 +0200
Subject: [PATCH] nfs command: Fix possible buffer overflow

the nfs command stores the nfs filename in a fixed size buffer without
checking its length. Instead of using a static buffer use strdup() to
dynamically allocate a suitably sized buffer.

Reported-by: Neeraj Pal <neerajpal09@gmail.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/nfs.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index 591417e0de..440e410a83 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -148,7 +148,6 @@ static int	nfs_state;
 
 static char *nfs_filename;
 static char *nfs_path;
-static char nfs_path_buff[2048];
 
 static int net_store_fd;
 static struct net_connection *nfs_con;
@@ -522,11 +521,26 @@ static int nfs_readlink_reply(unsigned char *pkt, unsigned len)
 	path = (char *)data;
 
 	if (*path != '/') {
-		strcat(nfs_path, "/");
-		strncat(nfs_path, path, rlen);
+		char *n;
+
+		n = calloc(strlen(nfs_path) + sizeof('/') + rlen + 1, 1);
+		if (!n)
+			return -ENOMEM;
+
+		strcpy(n, nfs_path);
+		strcat(n, "/");
+		strncat(n, path, rlen);
+
+		free(nfs_path);
+		nfs_path = n;
 	} else {
+		free(nfs_path);
+
+		nfs_path = calloc(rlen + 1, 1);
+		if (!nfs_path)
+			return -ENOMEM;
+
 		memcpy(nfs_path, path, rlen);
-		nfs_path[rlen] = 0;
 	}
 	return 0;
 }
@@ -655,13 +669,13 @@ err_out:
 	nfs_err = ret;
 }
 
-static void nfs_start(char *p)
+static int nfs_start(char *p)
 {
 	debug("%s\n", __func__);
 
-	nfs_path = (char *)nfs_path_buff;
-
-	strcpy(nfs_path, p);
+	nfs_path = strdup(p);
+	if (nfs_path)
+		return -ENOMEM;
 
 	nfs_filename = basename (nfs_path);
 	nfs_path     = dirname (nfs_path);
@@ -671,6 +685,8 @@ static void nfs_start(char *p)
 	nfs_state = STATE_PRCLOOKUP_PROG_MOUNT_REQ;
 
 	nfs_send();
+
+	return 0;
 }
 
 static int do_nfs(int argc, char *argv[])
@@ -701,9 +717,9 @@ static int do_nfs(int argc, char *argv[])
 	}
 	net_udp_bind(nfs_con, 1000);
 
-	nfs_err = 0;
-
-	nfs_start(remotefile);
+	nfs_err = nfs_start(remotefile);
+	if (nfs_err)
+		goto err_udp;
 
 	while (nfs_state != STATE_DONE) {
 		if (ctrlc()) {
@@ -727,6 +743,9 @@ err_udp:
 
 	printf("\n");
 
+	free(nfs_path);
+	nfs_path = NULL;
+
 	return nfs_err == 0 ? 0 : 1;
 }
 
-- 
2.29.2


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


  reply	other threads:[~2021-05-07  8:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-17 18:52 Neeraj Pal
2021-05-07  8:41 ` Sascha Hauer [this message]
2021-05-10 11:08   ` Neeraj Pal
2021-05-10 13:18     ` Neeraj Pal
2021-05-11  8:58     ` Sascha Hauer
2021-05-11 18:06       ` Neeraj Pal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210507084102.GU19819@pengutronix.de \
    --to=sha@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=neerajpal09@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox