[quagga-dev 10570] Re: ospfd, new_msg_lsa_change_notify: looks like a buffer overflow

Charlet, Ricky ricky.charlet at hp.com
Wed Jul 3 21:38:12 BST 2013


Thanks Alexis and David.

Lookin forward to the fix.  I'm just glad to have been one of the "1000 pairs of eyes"

--
Ricky Charlet
Software Dev / Routing Dude: Aries team, Roseville CA
ricky.charlet at hp.com
USA: 916.785.2090


-----Original Message-----
From: equinox at diac24.net [mailto:equinox at diac24.net] On Behalf Of David Lamparter
Sent: Wednesday, July 03, 2013 6:22 AM
To: Charlet, Ricky
Cc: quagga-dev at lists.quagga.net
Subject: Re: [quagga-dev 10567] Re: ospfd, new_msg_lsa_change_notify: looks like a buffer overflow

Hi Ricky,


it indeed looks like you found a possibly exploitable buffer overflow.
I'm looking into it and there will probably be a 0.99.22.2 out in a few days (which needs to be done anyway due to VU#229804).

On Tue, Jul 02, 2013 at 06:10:31PM +0000, Charlet, Ricky wrote:
> [...]
> I setup a network that, upon a single network event, sent over router 
> LSAs representing 150 networks. The actual traffic broke down into 
> about 100 ls-update packets, many of which are len=1480, and some of 
> which are ip-fragmented. The ls-updates tend to grow in size. The 
> largest (re-assembled) one was the last one at 1840 bytes.
> 
> While processing the received LSAs, we crash with  gdb backtrace 
> points to memcpy called from new_msg_lsa_change_notify. By code 
> review, I see that we memcpy into a buffer with a length we learned 
> from the input, not governed by the length of the available buffer. In 
> my patch, I suggest that we govern the memcpy by the length of the 
> available buffer.

That would indeed address the issue, however I'd like to look into dynamically allocating the buffer as an alternative.  Though for obvious reasons, that's only useful if the remainder of the code handles large packets well...

> <patch>
> +++ ospfd/ospf_api.c    (working copy)
> -  memcpy (&nmsg->data, data, ntohs (data->length));
> +  memcpy (&nmsg->data, data, sizeof(struct lsa_header));
>
> I tested the above, passing in the same large router-lsa's and it 
> survives/passes.
> 
> Sssooo.... I'm not one to pretend I understand what 'clients' might be 
> interested in this lsa-update message. I looked at the ospfclient.c:
> lsa_update_callback() and it apparently does not care about anything 
> beyond the header.  Certianly, in our implementation, we have no
> clients at all listening for these messages.   Is it apprpriate to
> trunkate this ls-update message to the header only, or should a 
> solution be built for passing arbitrarily large messages to clients?

"Clients" in this case are things like traffic engineering protocols or high availability stuff.  Quagga's OSPF API allows them to read the LSDB and get notified on updates.

> Additionally, how about the code that is sending the ls-updates?  I 
> did not look into it myself. But we may be a poor citizen if we are 
> sending ls-updates which need to be reassembled into large buffers.

Well, OSPFv2 explicitly relies on IP fragmentation to carry large LSAs.
This happening as in your case was designed into the protocol...  for reference, Cisco has some experiences with this too:
 https://supportforums.cisco.com/docs/DOC-14722
(Not directly related but explains OSPF packet sizing vs. MTU)

Cheers,


-David




More information about the Quagga-dev mailing list