diff options
3 files changed, 161 insertions, 0 deletions
diff --git a/meta-networking/recipes-daemons/atftp/atftp/0001-fix-buffer-overflow-in-atftpd.patch b/meta-networking/recipes-daemons/atftp/atftp/0001-fix-buffer-overflow-in-atftpd.patch new file mode 100644 index 0000000000..88794aa7ab --- /dev/null +++ b/meta-networking/recipes-daemons/atftp/atftp/0001-fix-buffer-overflow-in-atftpd.patch | |||
| @@ -0,0 +1,111 @@ | |||
| 1 | From d255bf90834fb45be52decf9bc0b4fb46c90f205 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Martin Dummer <md11@users.sourceforge.net> | ||
| 3 | Date: Sun, 12 Sep 2021 22:52:26 +0200 | ||
| 4 | Subject: [PATCH] fix buffer overflow in atftpd | ||
| 5 | |||
| 6 | Andreas B. Mundt <andi@debian.org> reports: | ||
| 7 | |||
| 8 | I've found a problem in atftpd that might be relevant for security. | ||
| 9 | The daemon can be crashed by any client sending a crafted combination | ||
| 10 | of TFTP options to the server. As TFTP is usually only used in the LAN, | ||
| 11 | it's probably not too dramatic. | ||
| 12 | |||
| 13 | Observations and how to reproduce the issue | ||
| 14 | =========================================== | ||
| 15 | |||
| 16 | Install bullseye packages and prepare tftp-root: | ||
| 17 | sudo apt install atftp atftpd | ||
| 18 | mkdir tmp | ||
| 19 | touch tmp/file.txt | ||
| 20 | |||
| 21 | Run server: | ||
| 22 | /usr/sbin/atftpd --user=$(id -un) --group=$(id -gn) --daemon --no-fork --trace \ | ||
| 23 | --logfile=/dev/stdout --verbose=7 --port 2000 tmp | ||
| 24 | |||
| 25 | Fetch file from client: | ||
| 26 | /usr/bin/atftp -g --trace --option "blksize 8" \ | ||
| 27 | --remote-file file.txt -l /dev/null 127.0.0.1 2000 | ||
| 28 | |||
| 29 | Crash server by adding another option to the tiny blksize: | ||
| 30 | /usr/bin/atftp -g --trace --option "blksize 8" --option "timeout 3" \ | ||
| 31 | --remote-file file.txt -l /dev/null 127.0.0.1 2000 | ||
| 32 | |||
| 33 | Analysis | ||
| 34 | ======== | ||
| 35 | |||
| 36 | The reason for the crash is a buffer overflow. The size of the buffer keeping the data | ||
| 37 | to be sent with every segment is calculated by adding 4 bytes to the blksize (for opcode | ||
| 38 | and block number). However, the same buffer is used for the OACK, which for a blksize=8 | ||
| 39 | overflows as soon as another option is set. | ||
| 40 | |||
| 41 | Signed-off-by: Martin Dummer <md11@users.sourceforge.net> | ||
| 42 | |||
| 43 | CVE: CVE-2021-41054 | ||
| 44 | Upstream-Status: Backport [https://github.com/madmartin/atftp/commit/d255bf90834fb45be52decf9bc0b4fb46c90f205.patch] | ||
| 45 | Signed-off-by: Ranjitsinh Rathod <ranjitsinh.rathod@kpit.com> | ||
| 46 | |||
| 47 | --- | ||
| 48 | tftpd_file.c | 34 ++++++++++++++++++++++++++++++---- | ||
| 49 | 1 file changed, 30 insertions(+), 4 deletions(-) | ||
| 50 | |||
| 51 | diff --git a/tftpd_file.c b/tftpd_file.c | ||
| 52 | index ff40e8d..37a0906 100644 | ||
| 53 | --- a/tftpd_file.c | ||
| 54 | +++ b/tftpd_file.c | ||
| 55 | @@ -168,11 +168,24 @@ int tftpd_receive_file(struct thread_data *data) | ||
| 56 | logger(LOG_DEBUG, "timeout option -> %d", timeout); | ||
| 57 | } | ||
| 58 | |||
| 59 | - /* blksize options */ | ||
| 60 | + /* | ||
| 61 | + * blksize option, must be the last option evaluated, | ||
| 62 | + * because data->data_buffer_size may be modified here, | ||
| 63 | + * and may be smaller than the buffer containing options | ||
| 64 | + */ | ||
| 65 | if ((result = opt_get_blksize(data->tftp_options)) > -1) | ||
| 66 | { | ||
| 67 | - if ((result < 8) || (result > 65464)) | ||
| 68 | + /* | ||
| 69 | + * If we receive more options, we have to make sure our buffer for | ||
| 70 | + * the OACK is not too small. Use the string representation of | ||
| 71 | + * the options here for simplicity, which puts us on the save side. | ||
| 72 | + * FIXME: Use independent buffers for OACK and data. | ||
| 73 | + */ | ||
| 74 | + opt_options_to_string(data->tftp_options, string, MAXLEN); | ||
| 75 | + if ((result < strlen(string)-2) || (result > 65464)) | ||
| 76 | { | ||
| 77 | + logger(LOG_NOTICE, "options <%s> require roughly a blksize of %d for the OACK.", | ||
| 78 | + string, strlen(string)-2); | ||
| 79 | tftp_send_error(sockfd, sa, EOPTNEG, data->data_buffer, data->data_buffer_size); | ||
| 80 | if (data->trace) | ||
| 81 | logger(LOG_DEBUG, "sent ERROR <code: %d, msg: %s>", EOPTNEG, | ||
| 82 | @@ -531,11 +544,24 @@ int tftpd_send_file(struct thread_data *data) | ||
| 83 | logger(LOG_INFO, "timeout option -> %d", timeout); | ||
| 84 | } | ||
| 85 | |||
| 86 | - /* blksize options */ | ||
| 87 | + /* | ||
| 88 | + * blksize option, must be the last option evaluated, | ||
| 89 | + * because data->data_buffer_size may be modified here, | ||
| 90 | + * and may be smaller than the buffer containing options | ||
| 91 | + */ | ||
| 92 | if ((result = opt_get_blksize(data->tftp_options)) > -1) | ||
| 93 | { | ||
| 94 | - if ((result < 8) || (result > 65464)) | ||
| 95 | + /* | ||
| 96 | + * If we receive more options, we have to make sure our buffer for | ||
| 97 | + * the OACK is not too small. Use the string representation of | ||
| 98 | + * the options here for simplicity, which puts us on the save side. | ||
| 99 | + * FIXME: Use independent buffers for OACK and data. | ||
| 100 | + */ | ||
| 101 | + opt_options_to_string(data->tftp_options, string, MAXLEN); | ||
| 102 | + if ((result < strlen(string)-2) || (result > 65464)) | ||
| 103 | { | ||
| 104 | + logger(LOG_NOTICE, "options <%s> require roughly a blksize of %d for the OACK.", | ||
| 105 | + string, strlen(string)-2); | ||
| 106 | tftp_send_error(sockfd, sa, EOPTNEG, data->data_buffer, data->data_buffer_size); | ||
| 107 | if (data->trace) | ||
| 108 | logger(LOG_DEBUG, "sent ERROR <code: %d, msg: %s>", EOPTNEG, | ||
| 109 | -- | ||
| 110 | 2.17.1 | ||
| 111 | |||
diff --git a/meta-networking/recipes-daemons/atftp/atftp/0001-options.c-Proper-fix-for-the-read-past-end-of-array.patch b/meta-networking/recipes-daemons/atftp/atftp/0001-options.c-Proper-fix-for-the-read-past-end-of-array.patch new file mode 100644 index 0000000000..310728aaca --- /dev/null +++ b/meta-networking/recipes-daemons/atftp/atftp/0001-options.c-Proper-fix-for-the-read-past-end-of-array.patch | |||
| @@ -0,0 +1,48 @@ | |||
| 1 | From 9cf799c40738722001552618518279e9f0ef62e5 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Simon Rettberg <simon.rettberg@rz.uni-freiburg.de> | ||
| 3 | Date: Wed, 10 Jan 2018 17:01:20 +0100 | ||
| 4 | Subject: [PATCH] options.c: Proper fix for the read-past-end-of-array | ||
| 5 | |||
| 6 | This properly fixes what commit:b3e36dd tried to do. | ||
| 7 | |||
| 8 | CVE: CVE-2021-46671 | ||
| 9 | Upstream-Status: Backport [https://github.com/madmartin/atftp/commit/9cf799c40738722001552618518279e9f0ef62e5.patch] | ||
| 10 | Signed-off-by: Ranjitsinh Rathod <ranjitsinh.rathod@kpit.com> | ||
| 11 | |||
| 12 | --- | ||
| 13 | options.c | 12 ++++++++++++ | ||
| 14 | 1 file changed, 12 insertions(+) | ||
| 15 | |||
| 16 | diff --git a/options.c b/options.c | ||
| 17 | index ee419c6..c716994 100644 | ||
| 18 | --- a/options.c | ||
| 19 | +++ b/options.c | ||
| 20 | @@ -43,6 +43,12 @@ int opt_parse_request(char *data, int data_size, struct tftp_opt *options) | ||
| 21 | struct tftphdr *tftp_data = (struct tftphdr *)data; | ||
| 22 | size_t size = data_size - sizeof(tftp_data->th_opcode); | ||
| 23 | |||
| 24 | + /* sanity check - requests always end in a null byte, | ||
| 25 | + * check to prevent argz_next from reading past the end of | ||
| 26 | + * data, as it doesn't do bounds checks */ | ||
| 27 | + if (data_size == 0 || data[data_size-1] != '\0') | ||
| 28 | + return ERR; | ||
| 29 | + | ||
| 30 | /* read filename */ | ||
| 31 | entry = argz_next(tftp_data->th_stuff, size, entry); | ||
| 32 | if (!entry) | ||
| 33 | @@ -79,6 +85,12 @@ int opt_parse_options(char *data, int data_size, struct tftp_opt *options) | ||
| 34 | struct tftphdr *tftp_data = (struct tftphdr *)data; | ||
| 35 | size_t size = data_size - sizeof(tftp_data->th_opcode); | ||
| 36 | |||
| 37 | + /* sanity check - options always end in a null byte, | ||
| 38 | + * check to prevent argz_next from reading past the end of | ||
| 39 | + * data, as it doesn't do bounds checks */ | ||
| 40 | + if (data_size == 0 || data[data_size-1] != '\0') | ||
| 41 | + return ERR; | ||
| 42 | + | ||
| 43 | while ((entry = argz_next(tftp_data->th_stuff, size, entry))) | ||
| 44 | { | ||
| 45 | tmp = entry; | ||
| 46 | -- | ||
| 47 | 2.17.1 | ||
| 48 | |||
diff --git a/meta-networking/recipes-daemons/atftp/atftp_0.7.2.bb b/meta-networking/recipes-daemons/atftp/atftp_0.7.2.bb index ddddb1b07a..32b776e578 100644 --- a/meta-networking/recipes-daemons/atftp/atftp_0.7.2.bb +++ b/meta-networking/recipes-daemons/atftp/atftp_0.7.2.bb | |||
| @@ -9,6 +9,8 @@ SRCREV = "52b71f0831dcbde508bd3a961d84abb80a62480f" | |||
| 9 | SRC_URI = "git://git.code.sf.net/p/atftp/code;branch=master \ | 9 | SRC_URI = "git://git.code.sf.net/p/atftp/code;branch=master \ |
| 10 | file://atftpd.init \ | 10 | file://atftpd.init \ |
| 11 | file://atftpd.service \ | 11 | file://atftpd.service \ |
| 12 | file://0001-options.c-Proper-fix-for-the-read-past-end-of-array.patch \ | ||
| 13 | file://0001-fix-buffer-overflow-in-atftpd.patch \ | ||
| 12 | " | 14 | " |
| 13 | SRC_URI_append_libc-musl = " file://0001-argz.h-fix-musl-compile-add-missing-defines.patch \ | 15 | SRC_URI_append_libc-musl = " file://0001-argz.h-fix-musl-compile-add-missing-defines.patch \ |
| 14 | file://0002-tftp.h-tftpd.h-fix-musl-compile-missing-include.patch \ | 16 | file://0002-tftp.h-tftpd.h-fix-musl-compile-missing-include.patch \ |
