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
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
|
From 2d59b26d3b554960c777003c431add89d018b0a6 Mon Sep 17 00:00:00 2001
From: Guy Harris <gharris@sonic.net>
Date: Tue, 17 Oct 2023 22:08:42 -0700
Subject: [PATCH] netscreen: do bounds checking for each byte of packet data.
Make sure each byte we add to the packet data from the file fits in the
buffer, rather than stuffing bytes into the buffer and checking
afterwards.
This prevents a buffer overflow.
Fixes #19404, which was filed as part of Trend Micro's Zero Day
Initiative as ZDI-CAN-22164.
While we're at it, expand a comment and make error messages give some
more detail.
Upstream-Status: Backport [https://gitlab.com/wireshark/wireshark/-/commit/3be1c99180a6fc48c34ae4bfc79bfd840b29ae3e]
CVE: CVE-2023-6175
Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
---
wiretap/netscreen.c | 125 +++++++++++++++++++++++++++++++++-----------
1 file changed, 94 insertions(+), 31 deletions(-)
diff --git a/wiretap/netscreen.c b/wiretap/netscreen.c
index 9ad825f..ffcb689 100644
--- a/wiretap/netscreen.c
+++ b/wiretap/netscreen.c
@@ -59,7 +59,12 @@ static gboolean netscreen_seek_read(wtap *wth, gint64 seek_off,
static gboolean parse_netscreen_packet(FILE_T fh, wtap_rec *rec,
Buffer* buf, char *line, int *err, gchar **err_info);
static int parse_single_hex_dump_line(char* rec, guint8 *buf,
- guint byte_offset);
+ guint byte_offset, guint pkt_len);
+
+/* Error returns from parse_single_hex_dump_line() */
+#define PARSE_LINE_INVALID_CHARACTER -1
+#define PARSE_LINE_NO_BYTES_SEEN -2
+#define PARSE_LINE_TOO_MANY_BYTES_SEEN -3
/* Returns TRUE if the line appears to be a line with protocol info.
Otherwise it returns FALSE. */
@@ -241,13 +246,40 @@ netscreen_seek_read(wtap *wth, gint64 seek_off, wtap_rec *rec, Buffer *buf,
2c 21 b6 d3 20 60 0c 8c 35 98 88 cf 20 91 0e a9 ,!...`..5.......
1d 0b ..
+ * The first line of a packet is in the form
+
+<secs>.<dsecs>: <iface>({i,o}) len=<length>:<llinfo>>
+ * where:
+ *
+ * <secs> and <dsecs> are a time stamp in seconds and deciseconds,
+ * giving the time since the firewall was booted;
+ *
+ * <iface> is the name of the interface on which the packet was
+ * received or on which it was transmitted;
+ *
+ * {i,o} is i for a received packet and o for a transmitted packet;
+ *
+ * <length> is the length of the packet on the network;
+ *
+ * <llinfo>, at least for Ethernet, appears to be a source MAC
+ * address, folowed by "->", folowed by a destination MAC
+ * address, followed by a sequence of Ethertypes, each
+ * preceded by a "/" (multiple Ethertypes if there are VLAN
+ * tags and the like), possibly followed by ", tag <tag>".
+ *
+ * Following that may be some "info lines", each of which is indented
+ * by 14 spaces, giving a dissection of the payload after the
+ * link-layer header.
+ *
+ * Following that is a hex/ASCII dump of the contents of the
+ * packet, with 16 octets per line.
*/
static gboolean
parse_netscreen_packet(FILE_T fh, wtap_rec *rec, Buffer* buf,
char *line, int *err, gchar **err_info)
{
- int pkt_len;
+ guint pkt_len;
int sec;
int dsec;
char cap_int[NETSCREEN_MAX_INT_NAME_LENGTH];
@@ -266,17 +298,12 @@ parse_netscreen_packet(FILE_T fh, wtap_rec *rec, Buffer* buf,
memset(cap_int, 0, sizeof(cap_int));
memset(cap_dst, 0, sizeof(cap_dst));
- if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9d:%12s->%12s/",
+ if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9u:%12s->%12s/",
&sec, &dsec, cap_int, direction, &pkt_len, cap_src, cap_dst) < 5) {
*err = WTAP_ERR_BAD_FILE;
*err_info = g_strdup("netscreen: Can't parse packet-header");
return -1;
}
- if (pkt_len < 0) {
- *err = WTAP_ERR_BAD_FILE;
- *err_info = g_strdup("netscreen: packet header has a negative packet length");
- return FALSE;
- }
if (pkt_len > WTAP_MAX_PACKET_SIZE_STANDARD) {
/*
* Probably a corrupt capture file; don't blow up trying
@@ -323,44 +350,71 @@ parse_netscreen_packet(FILE_T fh, wtap_rec *rec, Buffer* buf,
break;
}
- n = parse_single_hex_dump_line(p, pd, offset);
+ n = parse_single_hex_dump_line(p, pd, offset, pkt_len);
- /* the smallest packet has a length of 6 bytes, if
- * the first hex-data is less then check whether
- * it is a info-line and act accordingly
+ /*
+ * The smallest packet has a length of 6 bytes.
+ * If the first line either gets an error when
+ * parsed as hex data, or has fewer than 6
+ * bytes of hex data, check whether it's an
+ * info line by see if it has at least
+ * NETSCREEN_SPACES_ON_INFO_LINE spaces at the
+ * beginning.
+ *
+ * If it does, count this line and, if we have,
+ * so far, skipped no more than NETSCREEN_MAX_INFOLINES
+ * lines, skip this line.
*/
if (offset == 0 && n < 6) {
if (info_line(line)) {
+ /* Info line */
if (++i <= NETSCREEN_MAX_INFOLINES) {
+ /* Skip this line */
continue;
}
} else {
- *err = WTAP_ERR_BAD_FILE;
- *err_info = g_strdup("netscreen: cannot parse hex-data");
- return FALSE;
+ if (n >= 0) {
+ *err = WTAP_ERR_BAD_FILE;
+ *err_info = g_strdup("netscreen: first line of packet data has only %d hex bytes, < 6");
+ return FALSE;
+ }
+ /* Otherwise, fall through to report error */
}
}
/* If there is no more data and the line was not empty,
* then there must be an error in the file
*/
- if (n == -1) {
- *err = WTAP_ERR_BAD_FILE;
- *err_info = g_strdup("netscreen: cannot parse hex-data");
+ if (n < 0) {
+ switch (n) {
+
+ case PARSE_LINE_INVALID_CHARACTER:
+ *err = WTAP_ERR_BAD_FILE;
+ *err_info = g_strdup("netscreen: invalid character in hex data");
+ break;
+
+ case PARSE_LINE_NO_BYTES_SEEN:
+ *err = WTAP_ERR_BAD_FILE;
+ *err_info = g_strdup("netscreen: no hex bytes seen in hex data");
+ break;
+
+ case PARSE_LINE_TOO_MANY_BYTES_SEEN:
+ *err = WTAP_ERR_BAD_FILE;
+ *err_info = g_strdup("netscreen: number of hex bytes seen in hex data is greater than the packet length");
+ break;
+
+ default:
+ *err = WTAP_ERR_INTERNAL;
+ *err_info = g_strdup_printf("netscreen: unknown error %d from parse_single_hex_dump_line()", n);
+ break;
+ }
+
return FALSE;
}
/* Adjust the offset to the data that was just added to the buffer */
offset += n;
- /* If there was more hex-data than was announced in the len=x
- * header, then then there must be an error in the file
- */
- if (offset > pkt_len) {
- *err = WTAP_ERR_BAD_FILE;
- *err_info = g_strdup("netscreen: too much hex-data");
- return FALSE;
- }
}
/*
@@ -400,7 +454,7 @@ parse_netscreen_packet(FILE_T fh, wtap_rec *rec, Buffer* buf,
*
* Returns number of bytes successfully read, -1 if bad. */
static int
-parse_single_hex_dump_line(char* rec, guint8 *buf, guint byte_offset)
+parse_single_hex_dump_line(char* rec, guint8 *buf, guint byte_offset, guint pkt_len)
{
int num_items_scanned;
guint8 character;
@@ -419,7 +473,7 @@ parse_single_hex_dump_line(char* rec, guint8 *buf, guint byte_offset)
/* Nothing more to parse */
break;
} else
- return -1; /* not a hex digit, space before ASCII dump, or EOL */
+ return PARSE_LINE_INVALID_CHARACTER; /* not a hex digit, space before ASCII dump, or EOL */
byte <<= 4;
character = *rec++ & 0xFF;
if (character >= '0' && character <= '9')
@@ -429,7 +483,16 @@ parse_single_hex_dump_line(char* rec, guint8 *buf, guint byte_offset)
else if (character >= 'a' && character <= 'f')
byte += character - 'a' + 0xa;
else
- return -1; /* not a hex digit */
+ return PARSE_LINE_INVALID_CHARACTER; /* not a hex digit */
+
+ /* If there was more hex-data than was announced in the len=x
+ * header, then there must be an error in the file; quit
+ * now, as adding this byte will overflow the buffer.
+ */
+ if (byte_offset + num_items_scanned >= pkt_len) {
+ return PARSE_LINE_TOO_MANY_BYTES_SEEN;
+ }
+
buf[byte_offset + num_items_scanned] = byte;
character = *rec++ & 0xFF;
if (character == '\0' || character == '\r' || character == '\n') {
@@ -437,11 +500,11 @@ parse_single_hex_dump_line(char* rec, guint8 *buf, guint byte_offset)
break;
} else if (character != ' ') {
/* not space before ASCII dump */
- return -1;
+ return PARSE_LINE_INVALID_CHARACTER;
}
}
if (num_items_scanned == 0)
- return -1;
+ return PARSE_LINE_NO_BYTES_SEEN;
return num_items_scanned;
}
--
2.25.1
|
