Skip to content

Commit f9beb95

Browse files
committed
Merge branch 'sctp-size-validations'
Marcelo Ricardo Leitner says: ==================== sctp: add some size validations Ilja Van Sprundel reported that some size validations on inbound SCTP packets were missing. After some code review, I noticed two others that are all fixed here. Thanks Ilja for reporting this. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents 9ea3e52 + ef6c8d6 commit f9beb95

File tree

6 files changed

+58
-30
lines changed

6 files changed

+58
-30
lines changed

include/net/sctp/structs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ struct sctp_af {
461461
int saddr);
462462
void (*from_sk) (union sctp_addr *,
463463
struct sock *sk);
464-
void (*from_addr_param) (union sctp_addr *,
464+
bool (*from_addr_param) (union sctp_addr *,
465465
union sctp_addr_param *,
466466
__be16 port, int iif);
467467
int (*to_addr_param) (const union sctp_addr *,

net/sctp/bind_addr.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -270,22 +270,19 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
270270
rawaddr = (union sctp_addr_param *)raw_addr_list;
271271

272272
af = sctp_get_af_specific(param_type2af(param->type));
273-
if (unlikely(!af)) {
273+
if (unlikely(!af) ||
274+
!af->from_addr_param(&addr, rawaddr, htons(port), 0)) {
274275
retval = -EINVAL;
275-
sctp_bind_addr_clean(bp);
276-
break;
276+
goto out_err;
277277
}
278278

279-
af->from_addr_param(&addr, rawaddr, htons(port), 0);
280279
if (sctp_bind_addr_state(bp, &addr) != -1)
281280
goto next;
282281
retval = sctp_add_bind_addr(bp, &addr, sizeof(addr),
283282
SCTP_ADDR_SRC, gfp);
284-
if (retval) {
283+
if (retval)
285284
/* Can't finish building the list, clean up. */
286-
sctp_bind_addr_clean(bp);
287-
break;
288-
}
285+
goto out_err;
289286

290287
next:
291288
len = ntohs(param->length);
@@ -294,6 +291,12 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
294291
}
295292

296293
return retval;
294+
295+
out_err:
296+
if (retval)
297+
sctp_bind_addr_clean(bp);
298+
299+
return retval;
297300
}
298301

299302
/********************************************************************

net/sctp/input.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,7 +1131,8 @@ static struct sctp_association *__sctp_rcv_init_lookup(struct net *net,
11311131
if (!af)
11321132
continue;
11331133

1134-
af->from_addr_param(paddr, params.addr, sh->source, 0);
1134+
if (!af->from_addr_param(paddr, params.addr, sh->source, 0))
1135+
continue;
11351136

11361137
asoc = __sctp_lookup_association(net, laddr, paddr, transportp);
11371138
if (asoc)
@@ -1167,14 +1168,18 @@ static struct sctp_association *__sctp_rcv_asconf_lookup(
11671168
union sctp_addr_param *param;
11681169
union sctp_addr paddr;
11691170

1171+
if (ntohs(ch->length) < sizeof(*asconf) + sizeof(struct sctp_paramhdr))
1172+
return NULL;
1173+
11701174
/* Skip over the ADDIP header and find the Address parameter */
11711175
param = (union sctp_addr_param *)(asconf + 1);
11721176

11731177
af = sctp_get_af_specific(param_type2af(param->p.type));
11741178
if (unlikely(!af))
11751179
return NULL;
11761180

1177-
af->from_addr_param(&paddr, param, peer_port, 0);
1181+
if (af->from_addr_param(&paddr, param, peer_port, 0))
1182+
return NULL;
11781183

11791184
return __sctp_lookup_association(net, laddr, &paddr, transportp);
11801185
}
@@ -1245,7 +1250,7 @@ static struct sctp_association *__sctp_rcv_walk_lookup(struct net *net,
12451250

12461251
ch = (struct sctp_chunkhdr *)ch_end;
12471252
chunk_num++;
1248-
} while (ch_end < skb_tail_pointer(skb));
1253+
} while (ch_end + sizeof(*ch) < skb_tail_pointer(skb));
12491254

12501255
return asoc;
12511256
}

net/sctp/ipv6.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,15 +551,20 @@ static void sctp_v6_to_sk_daddr(union sctp_addr *addr, struct sock *sk)
551551
}
552552

553553
/* Initialize a sctp_addr from an address parameter. */
554-
static void sctp_v6_from_addr_param(union sctp_addr *addr,
554+
static bool sctp_v6_from_addr_param(union sctp_addr *addr,
555555
union sctp_addr_param *param,
556556
__be16 port, int iif)
557557
{
558+
if (ntohs(param->v6.param_hdr.length) < sizeof(struct sctp_ipv6addr_param))
559+
return false;
560+
558561
addr->v6.sin6_family = AF_INET6;
559562
addr->v6.sin6_port = port;
560563
addr->v6.sin6_flowinfo = 0; /* BUG */
561564
addr->v6.sin6_addr = param->v6.addr;
562565
addr->v6.sin6_scope_id = iif;
566+
567+
return true;
563568
}
564569

565570
/* Initialize an address parameter from a sctp_addr and return the length

net/sctp/protocol.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,19 @@ static void sctp_v4_to_sk_daddr(union sctp_addr *addr, struct sock *sk)
254254
}
255255

256256
/* Initialize a sctp_addr from an address parameter. */
257-
static void sctp_v4_from_addr_param(union sctp_addr *addr,
257+
static bool sctp_v4_from_addr_param(union sctp_addr *addr,
258258
union sctp_addr_param *param,
259259
__be16 port, int iif)
260260
{
261+
if (ntohs(param->v4.param_hdr.length) < sizeof(struct sctp_ipv4addr_param))
262+
return false;
263+
261264
addr->v4.sin_family = AF_INET;
262265
addr->v4.sin_port = port;
263266
addr->v4.sin_addr.s_addr = param->v4.addr.s_addr;
264267
memset(addr->v4.sin_zero, 0, sizeof(addr->v4.sin_zero));
268+
269+
return true;
265270
}
266271

267272
/* Initialize an address parameter from a sctp_addr and return the length

net/sctp/sm_make_chunk.c

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2166,9 +2166,16 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
21662166
break;
21672167

21682168
case SCTP_PARAM_SET_PRIMARY:
2169-
if (ep->asconf_enable)
2170-
break;
2171-
goto unhandled;
2169+
if (!ep->asconf_enable)
2170+
goto unhandled;
2171+
2172+
if (ntohs(param.p->length) < sizeof(struct sctp_addip_param) +
2173+
sizeof(struct sctp_paramhdr)) {
2174+
sctp_process_inv_paramlength(asoc, param.p,
2175+
chunk, err_chunk);
2176+
retval = SCTP_IERROR_ABORT;
2177+
}
2178+
break;
21722179

21732180
case SCTP_PARAM_HOST_NAME_ADDRESS:
21742181
/* Tell the peer, we won't support this param. */
@@ -2346,11 +2353,13 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
23462353

23472354
/* Process the initialization parameters. */
23482355
sctp_walk_params(param, peer_init, init_hdr.params) {
2349-
if (!src_match && (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
2350-
param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
2356+
if (!src_match &&
2357+
(param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
2358+
param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
23512359
af = sctp_get_af_specific(param_type2af(param.p->type));
2352-
af->from_addr_param(&addr, param.addr,
2353-
chunk->sctp_hdr->source, 0);
2360+
if (!af->from_addr_param(&addr, param.addr,
2361+
chunk->sctp_hdr->source, 0))
2362+
continue;
23542363
if (sctp_cmp_addr_exact(sctp_source(chunk), &addr))
23552364
src_match = 1;
23562365
}
@@ -2531,7 +2540,8 @@ static int sctp_process_param(struct sctp_association *asoc,
25312540
break;
25322541
do_addr_param:
25332542
af = sctp_get_af_specific(param_type2af(param.p->type));
2534-
af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0);
2543+
if (!af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0))
2544+
break;
25352545
scope = sctp_scope(peer_addr);
25362546
if (sctp_in_scope(net, &addr, scope))
25372547
if (!sctp_assoc_add_peer(asoc, &addr, gfp, SCTP_UNCONFIRMED))
@@ -2632,15 +2642,13 @@ static int sctp_process_param(struct sctp_association *asoc,
26322642
addr_param = param.v + sizeof(struct sctp_addip_param);
26332643

26342644
af = sctp_get_af_specific(param_type2af(addr_param->p.type));
2635-
if (af == NULL)
2645+
if (!af)
26362646
break;
26372647

2638-
af->from_addr_param(&addr, addr_param,
2639-
htons(asoc->peer.port), 0);
2648+
if (!af->from_addr_param(&addr, addr_param,
2649+
htons(asoc->peer.port), 0))
2650+
break;
26402651

2641-
/* if the address is invalid, we can't process it.
2642-
* XXX: see spec for what to do.
2643-
*/
26442652
if (!af->addr_valid(&addr, NULL, NULL))
26452653
break;
26462654

@@ -3054,7 +3062,8 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
30543062
if (unlikely(!af))
30553063
return SCTP_ERROR_DNS_FAILED;
30563064

3057-
af->from_addr_param(&addr, addr_param, htons(asoc->peer.port), 0);
3065+
if (!af->from_addr_param(&addr, addr_param, htons(asoc->peer.port), 0))
3066+
return SCTP_ERROR_DNS_FAILED;
30583067

30593068
/* ADDIP 4.2.1 This parameter MUST NOT contain a broadcast
30603069
* or multicast address.
@@ -3331,7 +3340,8 @@ static void sctp_asconf_param_success(struct sctp_association *asoc,
33313340

33323341
/* We have checked the packet before, so we do not check again. */
33333342
af = sctp_get_af_specific(param_type2af(addr_param->p.type));
3334-
af->from_addr_param(&addr, addr_param, htons(bp->port), 0);
3343+
if (!af->from_addr_param(&addr, addr_param, htons(bp->port), 0))
3344+
return;
33353345

33363346
switch (asconf_param->param_hdr.type) {
33373347
case SCTP_PARAM_ADD_IP:

0 commit comments

Comments
 (0)