aboutsummaryrefslogtreecommitdiffstats
path: root/meta-networking/recipes-daemons/atftp/atftp/0001-fix-buffer-overflow-in-atftpd.patch
blob: 88794aa7ab880585bfef5447a7dc30f5740d0be8 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
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