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
|