228 lines
8.1 KiB
Diff
228 lines
8.1 KiB
Diff
From 75e0ffcb42f3816e5f2fdef12f3c9ae906130b0c Mon Sep 17 00:00:00 2001
|
|
From: John Thacker <johnthacker@gmail.com>
|
|
Date: Sat, 24 Jun 2023 00:34:50 -0400
|
|
Subject: [PATCH] iscsi: Check bounds when extracting TargetAddress
|
|
|
|
Use tvb_ functions that do bounds checking when parsing the
|
|
TargetAddress string, instead of incrementing a pointer to an
|
|
extracted char* and sometimes accidentally overrunning the
|
|
string.
|
|
|
|
While we're there, go ahead and add support for IPv6 addresses.
|
|
|
|
Fix #19164
|
|
|
|
(backported from commit 94349bbdaeb384b12d554dd65e7be7ceb0e93d21)
|
|
---
|
|
epan/dissectors/packet-iscsi.c | 146 +++++++++++++++++----------------
|
|
1 file changed, 75 insertions(+), 71 deletions(-)
|
|
|
|
diff --git a/epan/dissectors/packet-iscsi.c b/epan/dissectors/packet-iscsi.c
|
|
index 031f07e5aa6..3b5d64de9fd 100644
|
|
--- a/epan/dissectors/packet-iscsi.c
|
|
+++ b/epan/dissectors/packet-iscsi.c
|
|
@@ -20,8 +20,6 @@
|
|
|
|
#include "config.h"
|
|
|
|
-#include <stdio.h>
|
|
-
|
|
#include <epan/packet.h>
|
|
#include <epan/prefs.h>
|
|
#include <epan/conversation.h>
|
|
@@ -29,6 +27,7 @@
|
|
#include "packet-scsi.h"
|
|
#include <epan/crc32-tvb.h>
|
|
#include <wsutil/crc32.h>
|
|
+#include <wsutil/inet_addr.h>
|
|
#include <wsutil/strtoi.h>
|
|
#include <wsutil/ws_roundup.h>
|
|
|
|
@@ -514,70 +513,81 @@ typedef struct _iscsi_conv_data {
|
|
dissector for the address/port that TargetAddress points to.
|
|
(it starts to be common to use redirectors to point to non-3260 ports)
|
|
*/
|
|
+static address null_address = ADDRESS_INIT_NONE;
|
|
+
|
|
static void
|
|
-iscsi_dissect_TargetAddress(packet_info *pinfo, tvbuff_t* tvb, proto_tree *tree, char *val, guint offset)
|
|
+iscsi_dissect_TargetAddress(packet_info *pinfo, tvbuff_t* tvb, proto_tree *tree, guint offset)
|
|
{
|
|
- address *addr = NULL;
|
|
+ address addr = ADDRESS_INIT_NONE;
|
|
guint16 port;
|
|
- char *value = wmem_strdup(pinfo->pool, val);
|
|
- char *p = NULL, *pgt = NULL;
|
|
-
|
|
- if (value[0] == '[') {
|
|
- /* this looks like an ipv6 address */
|
|
- p = strchr(value, ']');
|
|
- if (p != NULL) {
|
|
- *p = 0;
|
|
- p += 2; /* skip past "]:" */
|
|
-
|
|
- pgt = strchr(p, ',');
|
|
- if (pgt != NULL) {
|
|
- *pgt++ = 0;
|
|
- }
|
|
+ int colon_offset;
|
|
+ int end_offset;
|
|
+ char *ip_str, *port_str;
|
|
+
|
|
+ colon_offset = tvb_find_guint8(tvb, offset, -1, ':');
|
|
+ if (colon_offset == -1) {
|
|
+ /* RFC 7143 13.8 TargetAddress "If the TCP port is not specified,
|
|
+ * it is assumed to be the IANA-assigned default port for iSCSI",
|
|
+ * so nothing to do here.
|
|
+ */
|
|
+ return;
|
|
+ }
|
|
|
|
- /* can't handle ipv6 yet */
|
|
+ /* We found a colon, so there's at least one byte and this won't fail. */
|
|
+ if (tvb_get_guint8(tvb, offset) == '[') {
|
|
+ offset++;
|
|
+ /* could be an ipv6 address */
|
|
+ end_offset = tvb_find_guint8(tvb, offset, -1, ']');
|
|
+ if (end_offset == -1) {
|
|
+ return;
|
|
}
|
|
- } else {
|
|
- /* This is either a ipv4 address or a dns name */
|
|
- int i0,i1,i2,i3;
|
|
- if (sscanf(value, "%d.%d.%d.%d", &i0,&i1,&i2,&i3) == 4) {
|
|
- /* looks like a ipv4 address */
|
|
- p = strchr(value, ':');
|
|
- if (p != NULL) {
|
|
- char *addr_data;
|
|
-
|
|
- *p++ = 0;
|
|
-
|
|
- pgt = strchr(p, ',');
|
|
- if (pgt != NULL) {
|
|
- *pgt++ = 0;
|
|
- }
|
|
|
|
- addr_data = (char *) wmem_alloc(pinfo->pool, 4);
|
|
- addr_data[0] = i0;
|
|
- addr_data[1] = i1;
|
|
- addr_data[2] = i2;
|
|
- addr_data[3] = i3;
|
|
-
|
|
- addr = wmem_new(pinfo->pool, address);
|
|
- addr->type = AT_IPv4;
|
|
- addr->len = 4;
|
|
- addr->data = addr_data;
|
|
+ /* look for the colon before the port, if any */
|
|
+ colon_offset = tvb_find_guint8(tvb, end_offset, -1, ':');
|
|
+ if (colon_offset == -1) {
|
|
+ return;
|
|
+ }
|
|
|
|
- if (!ws_strtou16(p, NULL, &port)) {
|
|
- proto_tree_add_expert_format(tree, pinfo, &ei_iscsi_keyvalue_invalid,
|
|
- tvb, offset + (guint)strlen(value), (guint)strlen(p), "Invalid port: %s", p);
|
|
- }
|
|
- }
|
|
+ ws_in6_addr *ip6_addr = wmem_new(pinfo->pool, ws_in6_addr);
|
|
+ ip_str = tvb_get_string_enc(pinfo->pool, tvb, offset, end_offset - offset, ENC_ASCII);
|
|
+ if (ws_inet_pton6(ip_str, ip6_addr)) {
|
|
+ /* looks like a ipv6 address */
|
|
+ set_address(&addr, AT_IPv6, sizeof(ws_in6_addr), ip6_addr);
|
|
+ }
|
|
|
|
+ } else {
|
|
+ /* This is either a ipv4 address or a dns name */
|
|
+ ip_str = tvb_get_string_enc(pinfo->pool, tvb, offset, colon_offset - offset, ENC_ASCII);
|
|
+ ws_in4_addr *ip4_addr = wmem_new(pinfo->pool, ws_in4_addr);
|
|
+ if (ws_inet_pton4(ip_str, ip4_addr)) {
|
|
+ /* looks like a ipv4 address */
|
|
+ set_address(&addr, AT_IPv4, 4, ip4_addr);
|
|
}
|
|
+ /* else a DNS host name; we could, theoretically, try to use
|
|
+ * name resolution information in the capture to lookup the address.
|
|
+ */
|
|
}
|
|
|
|
+ /* Extract the port */
|
|
+ end_offset = tvb_find_guint8(tvb, colon_offset, -1, ',');
|
|
+ int port_len;
|
|
+ if (end_offset == -1) {
|
|
+ port_len = tvb_reported_length_remaining(tvb, colon_offset + 1);
|
|
+ } else {
|
|
+ port_len = end_offset - (colon_offset + 1);
|
|
+ }
|
|
+ port_str = tvb_get_string_enc(pinfo->pool, tvb, colon_offset + 1, port_len, ENC_ASCII);
|
|
+ if (!ws_strtou16(port_str, NULL, &port)) {
|
|
+ proto_tree_add_expert_format(tree, pinfo, &ei_iscsi_keyvalue_invalid,
|
|
+ tvb, colon_offset + 1, port_len, "Invalid port: %s", port_str);
|
|
+ return;
|
|
+ }
|
|
|
|
/* attach a conversation dissector to this address/port tuple */
|
|
- if (addr && !pinfo->fd->visited) {
|
|
+ if (!addresses_equal(&addr, &null_address) && !pinfo->fd->visited) {
|
|
conversation_t *conv;
|
|
|
|
- conv = conversation_new(pinfo->num, addr, addr, ENDPOINT_TCP, port, port, NO_ADDR2|NO_PORT2);
|
|
+ conv = conversation_new(pinfo->num, &addr, &null_address, ENDPOINT_TCP, port, 0, NO_ADDR2|NO_PORT2);
|
|
if (conv == NULL) {
|
|
return;
|
|
}
|
|
@@ -589,30 +599,24 @@ iscsi_dissect_TargetAddress(packet_info *pinfo, tvbuff_t* tvb, proto_tree *tree,
|
|
static gint
|
|
addTextKeys(packet_info *pinfo, proto_tree *tt, tvbuff_t *tvb, gint offset, guint32 text_len) {
|
|
const gint limit = offset + text_len;
|
|
+ tvbuff_t *keyvalue_tvb;
|
|
+ int len, value_offset;
|
|
|
|
while(offset < limit) {
|
|
- char *key = NULL, *value = NULL;
|
|
- gint len = tvb_strnlen(tvb, offset, limit - offset);
|
|
-
|
|
- if(len == -1) {
|
|
- len = limit - offset;
|
|
- } else {
|
|
- len = len + 1;
|
|
- }
|
|
-
|
|
- key = tvb_get_string_enc(pinfo->pool, tvb, offset, len, ENC_ASCII);
|
|
- if (key == NULL) {
|
|
- break;
|
|
- }
|
|
- value = strchr(key, '=');
|
|
- if (value == NULL) {
|
|
+ /* RFC 7143 6.1 Text Format: "Every key=value pair, including the
|
|
+ * last or only pair in a LTDS, MUST be followed by one null (0x00)
|
|
+ * delimiter.
|
|
+ */
|
|
+ proto_tree_add_item_ret_length(tt, hf_iscsi_KeyValue, tvb, offset, -1, ENC_ASCII, &len);
|
|
+ keyvalue_tvb = tvb_new_subset_length(tvb, offset, len);
|
|
+ value_offset = tvb_find_guint8(keyvalue_tvb, 0, len, '=');
|
|
+ if (value_offset == -1) {
|
|
break;
|
|
}
|
|
- *value++ = 0;
|
|
+ value_offset++;
|
|
|
|
- proto_tree_add_item(tt, hf_iscsi_KeyValue, tvb, offset, len, ENC_ASCII|ENC_NA);
|
|
- if (!strcmp(key, "TargetAddress")) {
|
|
- iscsi_dissect_TargetAddress(pinfo, tvb, tt, value, offset + (guint)strlen("TargetAddress") + 2);
|
|
+ if (tvb_strneql(keyvalue_tvb, 0, "TargetAddress=", strlen("TargetAddress=")) == 0) {
|
|
+ iscsi_dissect_TargetAddress(pinfo, keyvalue_tvb, tt, value_offset);
|
|
}
|
|
|
|
offset += len;
|
|
@@ -2943,7 +2947,7 @@ proto_register_iscsi(void)
|
|
},
|
|
{ &hf_iscsi_KeyValue,
|
|
{ "KeyValue", "iscsi.keyvalue",
|
|
- FT_STRING, BASE_NONE, NULL, 0,
|
|
+ FT_STRINGZ, BASE_NONE, NULL, 0,
|
|
"Key/value pair", HFILL }
|
|
},
|
|
{ &hf_iscsi_Text_F,
|
|
--
|
|
GitLab
|
|
|