Skip to content

Commit e942fb8

Browse files
guyharrisinfrastation
authored andcommitted
CVE-2017-12992/RIPng: Clean up bounds checking.
Do bounds checking as we access items. Scan the list of netinfo6 entries based on the supplied packet length, without taking the captured length into account; let the aforementioned bounds checking handle that. This fixes a buffer over-read discovered by Kamil Frankowicz. Add a test using the capture file supplied by the reporter(s).
1 parent db24063 commit e942fb8

File tree

4 files changed

+42
-31
lines changed

4 files changed

+42
-31
lines changed

Diff for: print-ripng.c

+40-31
Original file line numberDiff line numberDiff line change
@@ -110,65 +110,74 @@ ripng_print(netdissect_options *ndo, const u_char *dat, unsigned int length)
110110
{
111111
register const struct rip6 *rp = (const struct rip6 *)dat;
112112
register const struct netinfo6 *ni;
113-
register u_int amt;
114-
register u_int i;
115-
int j;
116-
int trunc;
117-
118-
if (ndo->ndo_snapend < dat)
119-
return;
120-
amt = ndo->ndo_snapend - dat;
121-
i = min(length, amt);
122-
if (i < (sizeof(struct rip6) - sizeof(struct netinfo6)))
123-
return;
124-
i -= (sizeof(struct rip6) - sizeof(struct netinfo6));
113+
unsigned int length_left;
114+
u_int j;
125115

116+
ND_TCHECK(rp->rip6_cmd);
126117
switch (rp->rip6_cmd) {
127118

128119
case RIP6_REQUEST:
129-
j = length / sizeof(*ni);
130-
if (j == 1
131-
&& rp->rip6_nets->rip6_metric == HOPCNT_INFINITY6
132-
&& IN6_IS_ADDR_UNSPECIFIED(&rp->rip6_nets->rip6_dest)) {
133-
ND_PRINT((ndo, " ripng-req dump"));
134-
break;
120+
length_left = length;
121+
if (length_left < (sizeof(struct rip6) - sizeof(struct netinfo6)))
122+
goto trunc;
123+
length_left -= (sizeof(struct rip6) - sizeof(struct netinfo6));
124+
j = length_left / sizeof(*ni);
125+
if (j == 1) {
126+
ND_TCHECK(rp->rip6_nets);
127+
if (rp->rip6_nets->rip6_metric == HOPCNT_INFINITY6
128+
&& IN6_IS_ADDR_UNSPECIFIED(&rp->rip6_nets->rip6_dest)) {
129+
ND_PRINT((ndo, " ripng-req dump"));
130+
break;
131+
}
135132
}
136-
if (j * sizeof(*ni) != length - 4)
137-
ND_PRINT((ndo, " ripng-req %d[%u]:", j, length));
133+
if (j * sizeof(*ni) != length_left)
134+
ND_PRINT((ndo, " ripng-req %u[%u]:", j, length));
138135
else
139-
ND_PRINT((ndo, " ripng-req %d:", j));
140-
trunc = ((i / sizeof(*ni)) * sizeof(*ni) != i);
141-
for (ni = rp->rip6_nets; i >= sizeof(*ni);
142-
i -= sizeof(*ni), ++ni) {
136+
ND_PRINT((ndo, " ripng-req %u:", j));
137+
for (ni = rp->rip6_nets; length_left >= sizeof(*ni);
138+
length_left -= sizeof(*ni), ++ni) {
139+
ND_TCHECK(*ni);
143140
if (ndo->ndo_vflag > 1)
144141
ND_PRINT((ndo, "\n\t"));
145142
else
146143
ND_PRINT((ndo, " "));
147144
rip6_entry_print(ndo, ni, 0);
148145
}
146+
if (length_left != 0)
147+
goto trunc;
149148
break;
150149
case RIP6_RESPONSE:
151-
j = length / sizeof(*ni);
152-
if (j * sizeof(*ni) != length - 4)
150+
length_left = length;
151+
if (length_left < (sizeof(struct rip6) - sizeof(struct netinfo6)))
152+
goto trunc;
153+
length_left -= (sizeof(struct rip6) - sizeof(struct netinfo6));
154+
j = length_left / sizeof(*ni);
155+
if (j * sizeof(*ni) != length_left)
153156
ND_PRINT((ndo, " ripng-resp %d[%u]:", j, length));
154157
else
155158
ND_PRINT((ndo, " ripng-resp %d:", j));
156-
trunc = ((i / sizeof(*ni)) * sizeof(*ni) != i);
157-
for (ni = rp->rip6_nets; i >= sizeof(*ni);
158-
i -= sizeof(*ni), ++ni) {
159+
for (ni = rp->rip6_nets; length_left >= sizeof(*ni);
160+
length_left -= sizeof(*ni), ++ni) {
161+
ND_TCHECK(*ni);
159162
if (ndo->ndo_vflag > 1)
160163
ND_PRINT((ndo, "\n\t"));
161164
else
162165
ND_PRINT((ndo, " "));
163166
rip6_entry_print(ndo, ni, ni->rip6_metric);
164167
}
165-
if (trunc)
166-
ND_PRINT((ndo, "[|ripng]"));
168+
if (length_left != 0)
169+
goto trunc;
167170
break;
168171
default:
169172
ND_PRINT((ndo, " ripng-%d ?? %u", rp->rip6_cmd, length));
170173
break;
171174
}
175+
ND_TCHECK(rp->rip6_vers);
172176
if (rp->rip6_vers != RIP6_VERSION)
173177
ND_PRINT((ndo, " [vers %d]", rp->rip6_vers));
178+
return;
179+
180+
trunc:
181+
ND_PRINT((ndo, "[|ripng]"));
182+
return;
174183
}

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,7 @@ isoclns-oobr isoclns-oobr.pcap isoclns-oobr.out
450450
nfs-attr-oobr nfs-attr-oobr.pcap nfs-attr-oobr.out
451451
decnet-oobr decnet-oobr.pcap decnet-oobr.out
452452
oobr_parse_elements oobr_parse_elements.pcap oobr_parse_elements.out
453+
hoobr_ripng_print hoobr_ripng_print.pcap hoobr_ripng_print.out
453454

454455
# bad packets from Wilfried Kirsch
455456
slip-bad-direction slip-bad-direction.pcap slip-bad-direction.out -ve

Diff for: tests/hoobr_ripng_print.out

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
IP 48.48.48.48.521 > 48.48.48.48.12336: [|ripng]

Diff for: tests/hoobr_ripng_print.pcap

88 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)