[quagga-dev 10597] Re: [PATCH] ospfd: CVE-2013-2236, stack overrun in apiserver
equinox at opensourcerouting.org
Mon Jul 15 13:45:34 BST 2013
On Tue, Jul 09, 2013 at 09:50:54AM +0200, Florian Weimer wrote:
> On 07/08/2013 11:06 PM, David Lamparter wrote:
> > @@ -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);
> > }
> I'm worried that this leaves data->length inconsistent with the outer
> PDU length. This might confuse further processing.
Yes, I understand your concern. Let me explain why I don't believe this
is an issue.
Essentially, the OSPF API is a fringe feature to begin with. I'm not
acutely aware of _any_ currently-maintained client using the interface
at all. I checked Debian, Gentoo and FreeBSD - none of these enable the
OSPF API by default.
Now, for the set of people who run Quagga with the OSPF API enabled,
there's probably some who enabled it for "shits 'n' giggles". Those
benefit from the patch.
That leaves people using the OSPF API for whatever purpose. They were
screwed before if they had large LSAs in their network. After the
patch, they get possibly truncated LSAs over the API. Can this cause
further problems? Yes. But it also allows handling the problem, as in
detecting a LSA was truncated. If the interface drops oversize LSAs, it
will simply become useless. With an oversize indication, it is at least
marginally useful since the client can _know_ whether the LSAs are
complete (which they will usually be, save some rare cases).
So, in the end, users of the OSPF API may indeed need to update theirs
software, but after doing that, the patch provides better functionality.
To make the OSPF API really usable, it needs to support 64k LSAs. The
simplest thing to do is probably using the stream_* functions. However,
due to the "fringeness" of the feature, I think this is a good place to
experiment with possible future new-IPC methods like protobuf.
More information about the Quagga-dev