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
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
|
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)
Upstream-Status: Backport [https://gitlab.com/wireshark/wireshark/-/commit/75e0ffcb42f3816e5f2fdef12f3c9ae906130b0c]
CVE: CVE-2023-3649
Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
---
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 8a80f49..08f44a8 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>
void proto_register_iscsi(void);
@@ -512,70 +511,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(wmem_packet_scope(), 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(wmem_packet_scope(), 4);
- addr_data[0] = i0;
- addr_data[1] = i1;
- addr_data[2] = i2;
- addr_data[3] = i3;
-
- addr = wmem_new(wmem_packet_scope(), 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;
}
@@ -587,30 +597,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(wmem_packet_scope(), 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;
@@ -2941,7 +2945,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,
--
2.25.1
|
