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
next prev parent 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