summaryrefslogtreecommitdiff
path: root/ospfd
diff options
context:
space:
mode:
authorDavid Lamparter <equinox@opensourcerouting.org>2013-07-08 23:05:28 +0200
committerDavid Lamparter <equinox@opensourcerouting.org>2013-07-28 16:13:10 +0200
commitc51443f4aa6b7f0b0d6ad5409ad7d4b215092443 (patch)
treeeffbe8695f7bfd0ed5261b08d5beddb66cceed64 /ospfd
parent78116ab6e1524815910658898620776ae5fd4d18 (diff)
ospfd: CVE-2013-2236, stack overrun in apiserver
the OSPF API-server (exporting the LSDB and allowing announcement of Opaque-LSAs) writes past the end of fixed on-stack buffers. This leads to an exploitable stack overflow. For this condition to occur, the following two conditions must be true: - Quagga is configured with --enable-opaque-lsa - ospfd is started with the "-a" command line option If either of these does not hold, the relevant code is not executed and the issue does not get triggered. Since the issue occurs on receiving large LSAs (larger than 1488 bytes), it is possible for this to happen during normal operation of a network. In particular, if there is an OSPF router with a large number of interfaces, the Router-LSA of that router may exceed 1488 bytes and trigger this, leading to an ospfd crash. For an attacker to exploit this, s/he must be able to inject valid LSAs into the OSPF domain. Any best-practice protection measure (using crypto authentication, restricting OSPF to internal interfaces, packet filtering protocol 89, etc.) will prevent exploitation. On top of that, remote (not on an OSPF-speaking network segment) attackers will have difficulties bringing up the adjacency needed to inject a LSA. This patch only performs minimal changes to remove the possibility of a stack overrun. The OSPF API in general is quite ugly and needs a rewrite. Reported-by: Ricky Charlet <ricky.charlet@hp.com> Cc: Florian Weimer <fweimer@redhat.com> Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Diffstat (limited to 'ospfd')
-rw-r--r--ospfd/ospf_api.c25
1 files changed, 18 insertions, 7 deletions
diff --git a/ospfd/ospf_api.c b/ospfd/ospf_api.c
index 74a49e3b..fae942ec 100644
--- a/ospfd/ospf_api.c
+++ b/ospfd/ospf_api.c
@@ -472,6 +472,9 @@ new_msg_register_event (u_int32_t seqnum, struct lsa_filter_type *filter)
emsg->filter.typemask = htons (filter->typemask);
emsg->filter.origin = filter->origin;
emsg->filter.num_areas = filter->num_areas;
+ if (len > sizeof (buf))
+ len = sizeof(buf);
+ /* API broken - missing memcpy to fill data */
return msg_new (MSG_REGISTER_EVENT, emsg, seqnum, len);
}
@@ -488,6 +491,9 @@ new_msg_sync_lsdb (u_int32_t seqnum, struct lsa_filter_type *filter)
smsg->filter.typemask = htons (filter->typemask);
smsg->filter.origin = filter->origin;
smsg->filter.num_areas = filter->num_areas;
+ if (len > sizeof (buf))
+ len = sizeof(buf);
+ /* API broken - missing memcpy to fill data */
return msg_new (MSG_SYNC_LSDB, smsg, seqnum, len);
}
@@ -501,13 +507,15 @@ new_msg_originate_request (u_int32_t seqnum,
int omsglen;
char buf[OSPF_API_MAX_MSG_SIZE];
- omsglen = sizeof (struct msg_originate_request) - sizeof (struct lsa_header)
- + ntohs (data->length);
-
omsg = (struct msg_originate_request *) buf;
omsg->ifaddr = ifaddr;
omsg->area_id = area_id;
- memcpy (&omsg->data, data, ntohs (data->length));
+
+ omsglen = ntohs (data->length);
+ if (omsglen > sizeof (buf) - offsetof (struct msg_originate_request, data))
+ omsglen = sizeof (buf) - offsetof (struct msg_originate_request, data);
+ memcpy (&omsg->data, data, omsglen);
+ omsglen += sizeof (struct msg_originate_request) - sizeof (struct lsa_header);
return msg_new (MSG_ORIGINATE_REQUEST, omsg, seqnum, omsglen);
}
@@ -627,13 +635,16 @@ new_msg_lsa_change_notify (u_char msgtype,
assert (data);
nmsg = (struct msg_lsa_change_notify *) buf;
- len = ntohs (data->length) + sizeof (struct msg_lsa_change_notify)
- - sizeof (struct lsa_header);
nmsg->ifaddr = ifaddr;
nmsg->area_id = area_id;
nmsg->is_self_originated = is_self_originated;
memset (&nmsg->pad, 0, sizeof (nmsg->pad));
- memcpy (&nmsg->data, data, ntohs (data->length));
+
+ len = ntohs (data->length);
+ if (len > sizeof (buf) - offsetof (struct msg_lsa_change_notify, data))
+ len = sizeof (buf) - offsetof (struct msg_lsa_change_notify, data);
+ memcpy (&nmsg->data, data, len);
+ len += sizeof (struct msg_lsa_change_notify) - sizeof (struct lsa_header);
return msg_new (msgtype, nmsg, seqnum, len);
}