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@xxxxxxxxxxxxxx>
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@xxxxxxxxx>
Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
