Package: netatalk / 2.2.5-2+deb9u1

115_CVE-2018-1160.patch Patch series | download
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
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
From 61c847c09f9df21692c2f8fee0060d764e453412 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Sat, 10 Nov 2018 13:40:04 +0100
Subject: [PATCH 1/2] CVE-2018-1160: libatalk/dsi: avoid double use of variable
 i

Signed-off-by: Ralph Boehme <slow@samba.org>
(backported from commit 67256322aa5a1fff01de471d6787d1d862678746)
---
 libatalk/dsi/dsi_opensess.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libatalk/dsi/dsi_opensess.c b/libatalk/dsi/dsi_opensess.c
index 2d7fd23b84..3a9e95ab6d 100644
--- a/libatalk/dsi/dsi_opensess.c
+++ b/libatalk/dsi/dsi_opensess.c
@@ -33,7 +33,9 @@ static void dsi_init_buffer(DSI *dsi)
 /* OpenSession. set up the connection */
 void dsi_opensession(DSI *dsi)
 {
-  u_int32_t i = 0; /* this serves double duty. it must be 4-bytes long */
+  size_t i = 0;
+  uint32_t servquant;
+  uint32_t replcsize;
   int offs;
 
   dsi_init_buffer(dsi);
@@ -62,21 +64,21 @@ void dsi_opensession(DSI *dsi)
   dsi->header.dsi_code = 0;
   /* dsi->header.dsi_command = DSIFUNC_OPEN;*/
 
-  dsi->cmdlen = 2 * (2 + sizeof(i)); /* length of data. dsi_send uses it. */
+  dsi->cmdlen = 2 * (2 + sizeof(uint32_t)); /* length of data. dsi_send uses it. */
 
   /* DSI Option Server Request Quantum */
   dsi->commands[0] = DSIOPT_SERVQUANT;
-  dsi->commands[1] = sizeof(i);
-  i = htonl(( dsi->server_quantum < DSI_SERVQUANT_MIN || 
+  dsi->commands[1] = sizeof(servquant);
+  servquant = htonl(( dsi->server_quantum < DSI_SERVQUANT_MIN ||
 	      dsi->server_quantum > DSI_SERVQUANT_MAX ) ? 
 	    DSI_SERVQUANT_DEF : dsi->server_quantum);
-  memcpy(dsi->commands + 2, &i, sizeof(i));
+  memcpy(dsi->commands + 2, &servquant, sizeof(servquant));
 
   /* AFP replaycache size option */
-  offs = 2 + sizeof(i);
+  offs = 2 + sizeof(replcsize);
   dsi->commands[offs] = DSIOPT_REPLCSIZE;
-  dsi->commands[offs+1] = sizeof(i);
-  i = htonl(REPLAYCACHE_SIZE);
-  memcpy(dsi->commands + offs + 2, &i, sizeof(i));
+  dsi->commands[offs+1] = sizeof(replcsize);
+  replcsize = htonl(REPLAYCACHE_SIZE);
+  memcpy(dsi->commands + offs + 2, &replcsize, sizeof(replcsize));
   dsi_send(dsi);
 }
-- 
2.17.2


From 9b9afa26cc211d532ed408ad3ebbcac40ae56cf2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Sat, 10 Nov 2018 13:41:43 +0100
Subject: [PATCH 2/2] CVE-2018-1160: libatalk/dsi: add correct bound checking
 to dsi_opensession

The memcpy

  memcpy(&dsi->attn_quantum, dsi->commands + i + 1, dsi->commands[i]);

trusted dsi->commands[i] to specify a size that fits into dsi->attn_quantum. The
sizeof attn_quantum is four bytes. A malicious client can send a dsi->command[i]
larger than 4 bytes to begin overwriting variables in the DSI struct.

dsi->command[i] is a single char in a char array which limits the amount of data
the attacker can overwrite in the DSI struct to 0xff. So for this to be useful
in an attack there needs to be something within the 0xff bytes that follow
attn_quantum. From dsi.h:

    uint32_t attn_quantum, datasize, server_quantum;
    uint16_t serverID, clientID;
    uint8_t  *commands; /* DSI recieve buffer */
    uint8_t  data[DSI_DATASIZ];    /* DSI reply buffer */

The commands pointer is a heap allocated pointer that is reused for every packet
received and sent. Using the memcpy, an attacker can overwrite this to point to
an address of their choice and then all subsequent AFP packets will be written
to that location.

If the attacker chose the preauth_switch buffer, overwriting the function
pointer there with functions pointers of his choice, he can invoke this
functions over the network,

Signed-off-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit b6895be1cb5b915254ee92c2150e309cd31ebff6)
---
 libatalk/dsi/dsi_opensess.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/libatalk/dsi/dsi_opensess.c b/libatalk/dsi/dsi_opensess.c
index 3a9e95ab6d..28282b7836 100644
--- a/libatalk/dsi/dsi_opensess.c
+++ b/libatalk/dsi/dsi_opensess.c
@@ -11,6 +11,7 @@
 #include <string.h>
 #include <sys/types.h>
 #include <stdlib.h>
+#include <inttypes.h>
 
 #include <atalk/dsi.h>
 #include <atalk/util.h>
@@ -37,6 +38,8 @@ void dsi_opensession(DSI *dsi)
   uint32_t servquant;
   uint32_t replcsize;
   int offs;
+  uint8_t cmd;
+  size_t option_len;
 
   dsi_init_buffer(dsi);
   if (setnonblock(dsi->socket, 1) < 0) {
@@ -45,17 +48,32 @@ void dsi_opensession(DSI *dsi)
   }
 
   /* parse options */
-  while (i < dsi->cmdlen) {
-    switch (dsi->commands[i++]) {
+  while (i + 1 < dsi->cmdlen) {
+    cmd = dsi->commands[i++];
+    option_len = dsi->commands[i++];
+
+    if (i + option_len > dsi->cmdlen) {
+      LOG(log_error, logtype_dsi, "option %"PRIu8" too large: %zu",
+          cmd, option_len);
+      exit(EXITERR_CLNT);
+    }
+
+    switch (cmd) {
     case DSIOPT_ATTNQUANT:
-      memcpy(&dsi->attn_quantum, dsi->commands + i + 1, dsi->commands[i]);
+      if (option_len != sizeof(dsi->attn_quantum)) {
+        LOG(log_error, logtype_dsi, "option %"PRIu8" bad length: %zu",
+            cmd, option_len);
+        exit(EXITERR_CLNT);
+      }
+      memcpy(&dsi->attn_quantum, &dsi->commands[i], option_len);
       dsi->attn_quantum = ntohl(dsi->attn_quantum);
 
     case DSIOPT_SERVQUANT: /* just ignore these */
     default:
-      i += dsi->commands[i] + 1; /* forward past length tag + length */
       break;
     }
+
+    i += option_len;
   }
 
   /* let the client know the server quantum. we don't use the
-- 
2.17.2