Skip to content

Commit 9de7922

Browse files
Daniel Borkmanndavem330
Daniel Borkmann
authored andcommitted
net: sctp: fix skb_over_panic when receiving malformed ASCONF chunks
Commit 6f4c618 ("SCTP : Add paramters validity check for ASCONF chunk") added basic verification of ASCONF chunks, however, it is still possible to remotely crash a server by sending a special crafted ASCONF chunk, even up to pre 2.6.12 kernels: skb_over_panic: text:ffffffffa01ea1c3 len:31056 put:30768 head:ffff88011bd81800 data:ffff88011bd81800 tail:0x7950 end:0x440 dev:<NULL> ------------[ cut here ]------------ kernel BUG at net/core/skbuff.c:129! [...] Call Trace: <IRQ> [<ffffffff8144fb1c>] skb_put+0x5c/0x70 [<ffffffffa01ea1c3>] sctp_addto_chunk+0x63/0xd0 [sctp] [<ffffffffa01eadaf>] sctp_process_asconf+0x1af/0x540 [sctp] [<ffffffff8152d025>] ? _read_unlock_bh+0x15/0x20 [<ffffffffa01e0038>] sctp_sf_do_asconf+0x168/0x240 [sctp] [<ffffffffa01e3751>] sctp_do_sm+0x71/0x1210 [sctp] [<ffffffff8147645d>] ? fib_rules_lookup+0xad/0xf0 [<ffffffffa01e6b22>] ? sctp_cmp_addr_exact+0x32/0x40 [sctp] [<ffffffffa01e8393>] sctp_assoc_bh_rcv+0xd3/0x180 [sctp] [<ffffffffa01ee986>] sctp_inq_push+0x56/0x80 [sctp] [<ffffffffa01fcc42>] sctp_rcv+0x982/0xa10 [sctp] [<ffffffffa01d5123>] ? ipt_local_in_hook+0x23/0x28 [iptable_filter] [<ffffffff8148bdc9>] ? nf_iterate+0x69/0xb0 [<ffffffff81496d10>] ? ip_local_deliver_finish+0x0/0x2d0 [<ffffffff8148bf86>] ? nf_hook_slow+0x76/0x120 [<ffffffff81496d10>] ? ip_local_deliver_finish+0x0/0x2d0 [<ffffffff81496ded>] ip_local_deliver_finish+0xdd/0x2d0 [<ffffffff81497078>] ip_local_deliver+0x98/0xa0 [<ffffffff8149653d>] ip_rcv_finish+0x12d/0x440 [<ffffffff81496ac5>] ip_rcv+0x275/0x350 [<ffffffff8145c88b>] __netif_receive_skb+0x4ab/0x750 [<ffffffff81460588>] netif_receive_skb+0x58/0x60 This can be triggered e.g., through a simple scripted nmap connection scan injecting the chunk after the handshake, for example, ... -------------- INIT[ASCONF; ASCONF_ACK] -------------> <----------- INIT-ACK[ASCONF; ASCONF_ACK] ------------ -------------------- COOKIE-ECHO --------------------> <-------------------- COOKIE-ACK --------------------- ------------------ ASCONF; UNKNOWN ------------------> ... where ASCONF chunk of length 280 contains 2 parameters ... 1) Add IP address parameter (param length: 16) 2) Add/del IP address parameter (param length: 255) ... followed by an UNKNOWN chunk of e.g. 4 bytes. Here, the Address Parameter in the ASCONF chunk is even missing, too. This is just an example and similarly-crafted ASCONF chunks could be used just as well. The ASCONF chunk passes through sctp_verify_asconf() as all parameters passed sanity checks, and after walking, we ended up successfully at the chunk end boundary, and thus may invoke sctp_process_asconf(). Parameter walking is done with WORD_ROUND() to take padding into account. In sctp_process_asconf()'s TLV processing, we may fail in sctp_process_asconf_param() e.g., due to removal of the IP address that is also the source address of the packet containing the ASCONF chunk, and thus we need to add all TLVs after the failure to our ASCONF response to remote via helper function sctp_add_asconf_response(), which basically invokes a sctp_addto_chunk() adding the error parameters to the given skb. When walking to the next parameter this time, we proceed with ... length = ntohs(asconf_param->param_hdr.length); asconf_param = (void *)asconf_param + length; ... instead of the WORD_ROUND()'ed length, thus resulting here in an off-by-one that leads to reading the follow-up garbage parameter length of 12336, and thus throwing an skb_over_panic for the reply when trying to sctp_addto_chunk() next time, which implicitly calls the skb_put() with that length. Fix it by using sctp_walk_params() [ which is also used in INIT parameter processing ] macro in the verification *and* in ASCONF processing: it will make sure we don't spill over, that we walk parameters WORD_ROUND()'ed. Moreover, we're being more defensive and guard against unknown parameter types and missized addresses. Joint work with Vlad Yasevich. Fixes: b896b82 ("[SCTP] ADDIP: Support for processing incoming ASCONF_ACK chunks.") Signed-off-by: Daniel Borkmann <dborkman@redhat.com> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com> Acked-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent b838b4a commit 9de7922

File tree

3 files changed

+60
-63
lines changed

3 files changed

+60
-63
lines changed

Diff for: include/net/sctp/sm.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,9 @@ struct sctp_chunk *sctp_make_asconf_update_ip(struct sctp_association *,
248248
int, __be16);
249249
struct sctp_chunk *sctp_make_asconf_set_prim(struct sctp_association *asoc,
250250
union sctp_addr *addr);
251-
int sctp_verify_asconf(const struct sctp_association *asoc,
252-
struct sctp_paramhdr *param_hdr, void *chunk_end,
253-
struct sctp_paramhdr **errp);
251+
bool sctp_verify_asconf(const struct sctp_association *asoc,
252+
struct sctp_chunk *chunk, bool addr_param_needed,
253+
struct sctp_paramhdr **errp);
254254
struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc,
255255
struct sctp_chunk *asconf);
256256
int sctp_process_asconf_ack(struct sctp_association *asoc,

Diff for: net/sctp/sm_make_chunk.c

+55-44
Original file line numberDiff line numberDiff line change
@@ -3110,50 +3110,63 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
31103110
return SCTP_ERROR_NO_ERROR;
31113111
}
31123112

3113-
/* Verify the ASCONF packet before we process it. */
3114-
int sctp_verify_asconf(const struct sctp_association *asoc,
3115-
struct sctp_paramhdr *param_hdr, void *chunk_end,
3116-
struct sctp_paramhdr **errp) {
3117-
sctp_addip_param_t *asconf_param;
3113+
/* Verify the ASCONF packet before we process it. */
3114+
bool sctp_verify_asconf(const struct sctp_association *asoc,
3115+
struct sctp_chunk *chunk, bool addr_param_needed,
3116+
struct sctp_paramhdr **errp)
3117+
{
3118+
sctp_addip_chunk_t *addip = (sctp_addip_chunk_t *) chunk->chunk_hdr;
31183119
union sctp_params param;
3119-
int length, plen;
3120-
3121-
param.v = (sctp_paramhdr_t *) param_hdr;
3122-
while (param.v <= chunk_end - sizeof(sctp_paramhdr_t)) {
3123-
length = ntohs(param.p->length);
3124-
*errp = param.p;
3120+
bool addr_param_seen = false;
31253121

3126-
if (param.v > chunk_end - length ||
3127-
length < sizeof(sctp_paramhdr_t))
3128-
return 0;
3122+
sctp_walk_params(param, addip, addip_hdr.params) {
3123+
size_t length = ntohs(param.p->length);
31293124

3125+
*errp = param.p;
31303126
switch (param.p->type) {
3127+
case SCTP_PARAM_ERR_CAUSE:
3128+
break;
3129+
case SCTP_PARAM_IPV4_ADDRESS:
3130+
if (length != sizeof(sctp_ipv4addr_param_t))
3131+
return false;
3132+
addr_param_seen = true;
3133+
break;
3134+
case SCTP_PARAM_IPV6_ADDRESS:
3135+
if (length != sizeof(sctp_ipv6addr_param_t))
3136+
return false;
3137+
addr_param_seen = true;
3138+
break;
31313139
case SCTP_PARAM_ADD_IP:
31323140
case SCTP_PARAM_DEL_IP:
31333141
case SCTP_PARAM_SET_PRIMARY:
3134-
asconf_param = (sctp_addip_param_t *)param.v;
3135-
plen = ntohs(asconf_param->param_hdr.length);
3136-
if (plen < sizeof(sctp_addip_param_t) +
3137-
sizeof(sctp_paramhdr_t))
3138-
return 0;
3142+
/* In ASCONF chunks, these need to be first. */
3143+
if (addr_param_needed && !addr_param_seen)
3144+
return false;
3145+
length = ntohs(param.addip->param_hdr.length);
3146+
if (length < sizeof(sctp_addip_param_t) +
3147+
sizeof(sctp_paramhdr_t))
3148+
return false;
31393149
break;
31403150
case SCTP_PARAM_SUCCESS_REPORT:
31413151
case SCTP_PARAM_ADAPTATION_LAYER_IND:
31423152
if (length != sizeof(sctp_addip_param_t))
3143-
return 0;
3144-
3153+
return false;
31453154
break;
31463155
default:
3147-
break;
3156+
/* This is unkown to us, reject! */
3157+
return false;
31483158
}
3149-
3150-
param.v += WORD_ROUND(length);
31513159
}
31523160

3153-
if (param.v != chunk_end)
3154-
return 0;
3161+
/* Remaining sanity checks. */
3162+
if (addr_param_needed && !addr_param_seen)
3163+
return false;
3164+
if (!addr_param_needed && addr_param_seen)
3165+
return false;
3166+
if (param.v != chunk->chunk_end)
3167+
return false;
31553168

3156-
return 1;
3169+
return true;
31573170
}
31583171

31593172
/* Process an incoming ASCONF chunk with the next expected serial no. and
@@ -3162,16 +3175,17 @@ int sctp_verify_asconf(const struct sctp_association *asoc,
31623175
struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc,
31633176
struct sctp_chunk *asconf)
31643177
{
3178+
sctp_addip_chunk_t *addip = (sctp_addip_chunk_t *) asconf->chunk_hdr;
3179+
bool all_param_pass = true;
3180+
union sctp_params param;
31653181
sctp_addiphdr_t *hdr;
31663182
union sctp_addr_param *addr_param;
31673183
sctp_addip_param_t *asconf_param;
31683184
struct sctp_chunk *asconf_ack;
3169-
31703185
__be16 err_code;
31713186
int length = 0;
31723187
int chunk_len;
31733188
__u32 serial;
3174-
int all_param_pass = 1;
31753189

31763190
chunk_len = ntohs(asconf->chunk_hdr->length) - sizeof(sctp_chunkhdr_t);
31773191
hdr = (sctp_addiphdr_t *)asconf->skb->data;
@@ -3199,38 +3213,35 @@ struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc,
31993213
goto done;
32003214

32013215
/* Process the TLVs contained within the ASCONF chunk. */
3202-
while (chunk_len > 0) {
3216+
sctp_walk_params(param, addip, addip_hdr.params) {
3217+
/* Skip preceeding address parameters. */
3218+
if (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
3219+
param.p->type == SCTP_PARAM_IPV6_ADDRESS)
3220+
continue;
3221+
32033222
err_code = sctp_process_asconf_param(asoc, asconf,
3204-
asconf_param);
3223+
param.addip);
32053224
/* ADDIP 4.1 A7)
32063225
* If an error response is received for a TLV parameter,
32073226
* all TLVs with no response before the failed TLV are
32083227
* considered successful if not reported. All TLVs after
32093228
* the failed response are considered unsuccessful unless
32103229
* a specific success indication is present for the parameter.
32113230
*/
3212-
if (SCTP_ERROR_NO_ERROR != err_code)
3213-
all_param_pass = 0;
3214-
3231+
if (err_code != SCTP_ERROR_NO_ERROR)
3232+
all_param_pass = false;
32153233
if (!all_param_pass)
3216-
sctp_add_asconf_response(asconf_ack,
3217-
asconf_param->crr_id, err_code,
3218-
asconf_param);
3234+
sctp_add_asconf_response(asconf_ack, param.addip->crr_id,
3235+
err_code, param.addip);
32193236

32203237
/* ADDIP 4.3 D11) When an endpoint receiving an ASCONF to add
32213238
* an IP address sends an 'Out of Resource' in its response, it
32223239
* MUST also fail any subsequent add or delete requests bundled
32233240
* in the ASCONF.
32243241
*/
3225-
if (SCTP_ERROR_RSRC_LOW == err_code)
3242+
if (err_code == SCTP_ERROR_RSRC_LOW)
32263243
goto done;
3227-
3228-
/* Move to the next ASCONF param. */
3229-
length = ntohs(asconf_param->param_hdr.length);
3230-
asconf_param = (void *)asconf_param + length;
3231-
chunk_len -= length;
32323244
}
3233-
32343245
done:
32353246
asoc->peer.addip_serial++;
32363247

Diff for: net/sctp/sm_statefuns.c

+2-16
Original file line numberDiff line numberDiff line change
@@ -3591,9 +3591,7 @@ sctp_disposition_t sctp_sf_do_asconf(struct net *net,
35913591
struct sctp_chunk *asconf_ack = NULL;
35923592
struct sctp_paramhdr *err_param = NULL;
35933593
sctp_addiphdr_t *hdr;
3594-
union sctp_addr_param *addr_param;
35953594
__u32 serial;
3596-
int length;
35973595

35983596
if (!sctp_vtag_verify(chunk, asoc)) {
35993597
sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_BAD_TAG,
@@ -3618,17 +3616,8 @@ sctp_disposition_t sctp_sf_do_asconf(struct net *net,
36183616
hdr = (sctp_addiphdr_t *)chunk->skb->data;
36193617
serial = ntohl(hdr->serial);
36203618

3621-
addr_param = (union sctp_addr_param *)hdr->params;
3622-
length = ntohs(addr_param->p.length);
3623-
if (length < sizeof(sctp_paramhdr_t))
3624-
return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
3625-
(void *)addr_param, commands);
3626-
36273619
/* Verify the ASCONF chunk before processing it. */
3628-
if (!sctp_verify_asconf(asoc,
3629-
(sctp_paramhdr_t *)((void *)addr_param + length),
3630-
(void *)chunk->chunk_end,
3631-
&err_param))
3620+
if (!sctp_verify_asconf(asoc, chunk, true, &err_param))
36323621
return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
36333622
(void *)err_param, commands);
36343623

@@ -3745,10 +3734,7 @@ sctp_disposition_t sctp_sf_do_asconf_ack(struct net *net,
37453734
rcvd_serial = ntohl(addip_hdr->serial);
37463735

37473736
/* Verify the ASCONF-ACK chunk before processing it. */
3748-
if (!sctp_verify_asconf(asoc,
3749-
(sctp_paramhdr_t *)addip_hdr->params,
3750-
(void *)asconf_ack->chunk_end,
3751-
&err_param))
3737+
if (!sctp_verify_asconf(asoc, asconf_ack, false, &err_param))
37523738
return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
37533739
(void *)err_param, commands);
37543740

0 commit comments

Comments
 (0)