-
Notifications
You must be signed in to change notification settings - Fork 682
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
atftp: Add fix for CVE-2021-41054 and CVE-2021-46671
Add patches to fix CVE-2021-41054 and CVE-2021-46671 issues Link: https://nvd.nist.gov/vuln/detail/CVE-2021-41054 Link: https://nvd.nist.gov/vuln/detail/CVE-2021-46671 Signed-off-by: Ranjitsinh Rathod <ranjitsinh.rathod@kpit.com> Signed-off-by: Ranjitsinh Rathod <ranjitsinhrathod1991@gmail.com> Signed-off-by: Armin Kuster <akuster808@gmail.com>
- Loading branch information
Showing
3 changed files
with
161 additions
and
0 deletions.
There are no files selected for viewing
111 changes: 111 additions & 0 deletions
111
meta-networking/recipes-daemons/atftp/atftp/0001-fix-buffer-overflow-in-atftpd.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
From d255bf90834fb45be52decf9bc0b4fb46c90f205 Mon Sep 17 00:00:00 2001 | ||
From: Martin Dummer <md11@users.sourceforge.net> | ||
Date: Sun, 12 Sep 2021 22:52:26 +0200 | ||
Subject: [PATCH] fix buffer overflow in atftpd | ||
|
||
Andreas B. Mundt <andi@debian.org> reports: | ||
|
||
I've found a problem in atftpd that might be relevant for security. | ||
The daemon can be crashed by any client sending a crafted combination | ||
of TFTP options to the server. As TFTP is usually only used in the LAN, | ||
it's probably not too dramatic. | ||
|
||
Observations and how to reproduce the issue | ||
=========================================== | ||
|
||
Install bullseye packages and prepare tftp-root: | ||
sudo apt install atftp atftpd | ||
mkdir tmp | ||
touch tmp/file.txt | ||
|
||
Run server: | ||
/usr/sbin/atftpd --user=$(id -un) --group=$(id -gn) --daemon --no-fork --trace \ | ||
--logfile=/dev/stdout --verbose=7 --port 2000 tmp | ||
|
||
Fetch file from client: | ||
/usr/bin/atftp -g --trace --option "blksize 8" \ | ||
--remote-file file.txt -l /dev/null 127.0.0.1 2000 | ||
|
||
Crash server by adding another option to the tiny blksize: | ||
/usr/bin/atftp -g --trace --option "blksize 8" --option "timeout 3" \ | ||
--remote-file file.txt -l /dev/null 127.0.0.1 2000 | ||
|
||
Analysis | ||
======== | ||
|
||
The reason for the crash is a buffer overflow. The size of the buffer keeping the data | ||
to be sent with every segment is calculated by adding 4 bytes to the blksize (for opcode | ||
and block number). However, the same buffer is used for the OACK, which for a blksize=8 | ||
overflows as soon as another option is set. | ||
|
||
Signed-off-by: Martin Dummer <md11@users.sourceforge.net> | ||
|
||
CVE: CVE-2021-41054 | ||
Upstream-Status: Backport [https://github.com/madmartin/atftp/commit/d255bf90834fb45be52decf9bc0b4fb46c90f205.patch] | ||
Signed-off-by: Ranjitsinh Rathod <ranjitsinh.rathod@kpit.com> | ||
|
||
--- | ||
tftpd_file.c | 34 ++++++++++++++++++++++++++++++---- | ||
1 file changed, 30 insertions(+), 4 deletions(-) | ||
|
||
diff --git a/tftpd_file.c b/tftpd_file.c | ||
index ff40e8d..37a0906 100644 | ||
--- a/tftpd_file.c | ||
+++ b/tftpd_file.c | ||
@@ -168,11 +168,24 @@ int tftpd_receive_file(struct thread_data *data) | ||
logger(LOG_DEBUG, "timeout option -> %d", timeout); | ||
} | ||
|
||
- /* blksize options */ | ||
+ /* | ||
+ * blksize option, must be the last option evaluated, | ||
+ * because data->data_buffer_size may be modified here, | ||
+ * and may be smaller than the buffer containing options | ||
+ */ | ||
if ((result = opt_get_blksize(data->tftp_options)) > -1) | ||
{ | ||
- if ((result < 8) || (result > 65464)) | ||
+ /* | ||
+ * If we receive more options, we have to make sure our buffer for | ||
+ * the OACK is not too small. Use the string representation of | ||
+ * the options here for simplicity, which puts us on the save side. | ||
+ * FIXME: Use independent buffers for OACK and data. | ||
+ */ | ||
+ opt_options_to_string(data->tftp_options, string, MAXLEN); | ||
+ if ((result < strlen(string)-2) || (result > 65464)) | ||
{ | ||
+ logger(LOG_NOTICE, "options <%s> require roughly a blksize of %d for the OACK.", | ||
+ string, strlen(string)-2); | ||
tftp_send_error(sockfd, sa, EOPTNEG, data->data_buffer, data->data_buffer_size); | ||
if (data->trace) | ||
logger(LOG_DEBUG, "sent ERROR <code: %d, msg: %s>", EOPTNEG, | ||
@@ -531,11 +544,24 @@ int tftpd_send_file(struct thread_data *data) | ||
logger(LOG_INFO, "timeout option -> %d", timeout); | ||
} | ||
|
||
- /* blksize options */ | ||
+ /* | ||
+ * blksize option, must be the last option evaluated, | ||
+ * because data->data_buffer_size may be modified here, | ||
+ * and may be smaller than the buffer containing options | ||
+ */ | ||
if ((result = opt_get_blksize(data->tftp_options)) > -1) | ||
{ | ||
- if ((result < 8) || (result > 65464)) | ||
+ /* | ||
+ * If we receive more options, we have to make sure our buffer for | ||
+ * the OACK is not too small. Use the string representation of | ||
+ * the options here for simplicity, which puts us on the save side. | ||
+ * FIXME: Use independent buffers for OACK and data. | ||
+ */ | ||
+ opt_options_to_string(data->tftp_options, string, MAXLEN); | ||
+ if ((result < strlen(string)-2) || (result > 65464)) | ||
{ | ||
+ logger(LOG_NOTICE, "options <%s> require roughly a blksize of %d for the OACK.", | ||
+ string, strlen(string)-2); | ||
tftp_send_error(sockfd, sa, EOPTNEG, data->data_buffer, data->data_buffer_size); | ||
if (data->trace) | ||
logger(LOG_DEBUG, "sent ERROR <code: %d, msg: %s>", EOPTNEG, | ||
-- | ||
2.17.1 | ||
|
48 changes: 48 additions & 0 deletions
48
...ecipes-daemons/atftp/atftp/0001-options.c-Proper-fix-for-the-read-past-end-of-array.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
From 9cf799c40738722001552618518279e9f0ef62e5 Mon Sep 17 00:00:00 2001 | ||
From: Simon Rettberg <simon.rettberg@rz.uni-freiburg.de> | ||
Date: Wed, 10 Jan 2018 17:01:20 +0100 | ||
Subject: [PATCH] options.c: Proper fix for the read-past-end-of-array | ||
|
||
This properly fixes what commit:b3e36dd tried to do. | ||
|
||
CVE: CVE-2021-46671 | ||
Upstream-Status: Backport [https://github.com/madmartin/atftp/commit/9cf799c40738722001552618518279e9f0ef62e5.patch] | ||
Signed-off-by: Ranjitsinh Rathod <ranjitsinh.rathod@kpit.com> | ||
|
||
--- | ||
options.c | 12 ++++++++++++ | ||
1 file changed, 12 insertions(+) | ||
|
||
diff --git a/options.c b/options.c | ||
index ee419c6..c716994 100644 | ||
--- a/options.c | ||
+++ b/options.c | ||
@@ -43,6 +43,12 @@ int opt_parse_request(char *data, int data_size, struct tftp_opt *options) | ||
struct tftphdr *tftp_data = (struct tftphdr *)data; | ||
size_t size = data_size - sizeof(tftp_data->th_opcode); | ||
|
||
+ /* sanity check - requests always end in a null byte, | ||
+ * check to prevent argz_next from reading past the end of | ||
+ * data, as it doesn't do bounds checks */ | ||
+ if (data_size == 0 || data[data_size-1] != '\0') | ||
+ return ERR; | ||
+ | ||
/* read filename */ | ||
entry = argz_next(tftp_data->th_stuff, size, entry); | ||
if (!entry) | ||
@@ -79,6 +85,12 @@ int opt_parse_options(char *data, int data_size, struct tftp_opt *options) | ||
struct tftphdr *tftp_data = (struct tftphdr *)data; | ||
size_t size = data_size - sizeof(tftp_data->th_opcode); | ||
|
||
+ /* sanity check - options always end in a null byte, | ||
+ * check to prevent argz_next from reading past the end of | ||
+ * data, as it doesn't do bounds checks */ | ||
+ if (data_size == 0 || data[data_size-1] != '\0') | ||
+ return ERR; | ||
+ | ||
while ((entry = argz_next(tftp_data->th_stuff, size, entry))) | ||
{ | ||
tmp = entry; | ||
-- | ||
2.17.1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters