|
1 This patch may be removed once Quagga is updated to 0.99.22.2 or |
|
2 later. |
|
3 |
|
4 |
|
5 From c51443f4aa6b7f0b0d6ad5409ad7d4b215092443 Mon Sep 17 00:00:00 2001 |
|
6 From: David Lamparter <[email protected]> |
|
7 Date: Mon, 8 Jul 2013 23:05:28 +0200 |
|
8 Subject: [PATCH] ospfd: CVE-2013-2236, stack overrun in apiserver |
|
9 |
|
10 the OSPF API-server (exporting the LSDB and allowing announcement of |
|
11 Opaque-LSAs) writes past the end of fixed on-stack buffers. This leads |
|
12 to an exploitable stack overflow. |
|
13 |
|
14 For this condition to occur, the following two conditions must be true: |
|
15 - Quagga is configured with --enable-opaque-lsa |
|
16 - ospfd is started with the "-a" command line option |
|
17 |
|
18 If either of these does not hold, the relevant code is not executed and |
|
19 the issue does not get triggered. |
|
20 |
|
21 Since the issue occurs on receiving large LSAs (larger than 1488 bytes), |
|
22 it is possible for this to happen during normal operation of a network. |
|
23 In particular, if there is an OSPF router with a large number of |
|
24 interfaces, the Router-LSA of that router may exceed 1488 bytes and |
|
25 trigger this, leading to an ospfd crash. |
|
26 |
|
27 For an attacker to exploit this, s/he must be able to inject valid LSAs |
|
28 into the OSPF domain. Any best-practice protection measure (using |
|
29 crypto authentication, restricting OSPF to internal interfaces, packet |
|
30 filtering protocol 89, etc.) will prevent exploitation. On top of that, |
|
31 remote (not on an OSPF-speaking network segment) attackers will have |
|
32 difficulties bringing up the adjacency needed to inject a LSA. |
|
33 |
|
34 This patch only performs minimal changes to remove the possibility of a |
|
35 stack overrun. The OSPF API in general is quite ugly and needs a |
|
36 rewrite. |
|
37 |
|
38 Reported-by: Ricky Charlet <[email protected]> |
|
39 Cc: Florian Weimer <[email protected]> |
|
40 Signed-off-by: David Lamparter <[email protected]> |
|
41 --- |
|
42 ospfd/ospf_api.c | 25 ++++++++++++++++++------- |
|
43 1 files changed, 19 insertions(+), 7 deletions(-) |
|
44 |
|
45 --- ospfd/ospf_api.c |
|
46 +++ ospfd/ospf_api.c |
|
47 @@ -21,6 +21,7 @@ |
|
48 */ |
|
49 |
|
50 #include <zebra.h> |
|
51 +#include <stddef.h> |
|
52 |
|
53 #ifdef SUPPORT_OSPF_API |
|
54 #ifndef HAVE_OPAQUE_LSA |
|
55 @@ -472,6 +473,9 @@ new_msg_register_event (u_int32_t seqnum |
|
56 emsg->filter.typemask = htons (filter->typemask); |
|
57 emsg->filter.origin = filter->origin; |
|
58 emsg->filter.num_areas = filter->num_areas; |
|
59 + if (len > sizeof (buf)) |
|
60 + len = sizeof(buf); |
|
61 + /* API broken - missing memcpy to fill data */ |
|
62 return msg_new (MSG_REGISTER_EVENT, emsg, seqnum, len); |
|
63 } |
|
64 |
|
65 @@ -488,6 +492,9 @@ new_msg_sync_lsdb (u_int32_t seqnum, str |
|
66 smsg->filter.typemask = htons (filter->typemask); |
|
67 smsg->filter.origin = filter->origin; |
|
68 smsg->filter.num_areas = filter->num_areas; |
|
69 + if (len > sizeof (buf)) |
|
70 + len = sizeof(buf); |
|
71 + /* API broken - missing memcpy to fill data */ |
|
72 return msg_new (MSG_SYNC_LSDB, smsg, seqnum, len); |
|
73 } |
|
74 |
|
75 @@ -501,13 +508,15 @@ new_msg_originate_request (u_int32_t seq |
|
76 int omsglen; |
|
77 char buf[OSPF_API_MAX_MSG_SIZE]; |
|
78 |
|
79 - omsglen = sizeof (struct msg_originate_request) - sizeof (struct lsa_header) |
|
80 - + ntohs (data->length); |
|
81 - |
|
82 omsg = (struct msg_originate_request *) buf; |
|
83 omsg->ifaddr = ifaddr; |
|
84 omsg->area_id = area_id; |
|
85 - memcpy (&omsg->data, data, ntohs (data->length)); |
|
86 + |
|
87 + omsglen = ntohs (data->length); |
|
88 + if (omsglen > sizeof (buf) - offsetof (struct msg_originate_request, data)) |
|
89 + omsglen = sizeof (buf) - offsetof (struct msg_originate_request, data); |
|
90 + memcpy (&omsg->data, data, omsglen); |
|
91 + omsglen += sizeof (struct msg_originate_request) - sizeof (struct lsa_header); |
|
92 |
|
93 return msg_new (MSG_ORIGINATE_REQUEST, omsg, seqnum, omsglen); |
|
94 } |
|
95 @@ -627,13 +636,16 @@ new_msg_lsa_change_notify (u_char msgtyp |
|
96 assert (data); |
|
97 |
|
98 nmsg = (struct msg_lsa_change_notify *) buf; |
|
99 - len = ntohs (data->length) + sizeof (struct msg_lsa_change_notify) |
|
100 - - sizeof (struct lsa_header); |
|
101 nmsg->ifaddr = ifaddr; |
|
102 nmsg->area_id = area_id; |
|
103 nmsg->is_self_originated = is_self_originated; |
|
104 memset (&nmsg->pad, 0, sizeof (nmsg->pad)); |
|
105 - memcpy (&nmsg->data, data, ntohs (data->length)); |
|
106 + |
|
107 + len = ntohs (data->length); |
|
108 + if (len > sizeof (buf) - offsetof (struct msg_lsa_change_notify, data)) |
|
109 + len = sizeof (buf) - offsetof (struct msg_lsa_change_notify, data); |
|
110 + memcpy (&nmsg->data, data, len); |
|
111 + len += sizeof (struct msg_lsa_change_notify) - sizeof (struct lsa_header); |
|
112 |
|
113 return msg_new (msgtype, nmsg, seqnum, len); |
|
114 } |