Attachment 14736 Details for Bug 13711 – Patch for master

[patch] Patch for master

CVE-2018-1160-master-01.patch (text/plain), 5.35 KB, created by Ralph Böhme on 2018-12-13 21:37:32 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2018-12-13 21:37:32 UTC

Size: 5.35 KB

patch

obsolete

>From 67256322aa5a1fff01de471d6787d1d862678746 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>
>Reviewed-by: HAT <hat@fa2.so-net.ne.jp>
>Reviewed-by: Andrew Stormont <andyjstormont@gmail.com>
>---
> 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 2945f9b185..85ed029472 100644
>--- a/libatalk/dsi/dsi_opensess.c
>+++ b/libatalk/dsi/dsi_opensess.c
>@@ -19,7 +19,9 @@
> /* OpenSession. set up the connection */
> void dsi_opensession(DSI *dsi)
> {
>-  uint32_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;
> 
>   if (setnonblock(dsi->socket, 1) < 0) {
>@@ -47,21 +49,21 @@ void dsi_opensession(DSI *dsi)
>   dsi->header.dsi_data.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 b6895be1cb5b915254ee92c2150e309cd31ebff6 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>
>Reviewed-by: HAT <hat@fa2.so-net.ne.jp>
>Reviewed-by: Andrew Stormont <andyjstormont@gmail.com>
>---
> libatalk/dsi/dsi_opensess.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
>diff --git a/libatalk/dsi/dsi_opensess.c b/libatalk/dsi/dsi_opensess.c
>index 85ed029472..193ab4c7f4 100644
>--- a/libatalk/dsi/dsi_opensess.c
>+++ b/libatalk/dsi/dsi_opensess.c
>@@ -23,6 +23,8 @@ void dsi_opensession(DSI *dsi)
>   uint32_t servquant;
>   uint32_t replcsize;
>   int offs;
>+  uint8_t cmd;
>+  size_t option_len;
> 
>   if (setnonblock(dsi->socket, 1) < 0) {
>       LOG(log_error, logtype_dsi, "dsi_opensession: setnonblock: %s", strerror(errno));
>@@ -30,17 +32,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
>

Actions: View

Attachments on bug 13711: 14735 | 14736 | 14737 | 14747 | 14748