|
Adam Tkac |
dd793e2 |
@@ -, +, @@
|
|
Adam Tkac |
dd793e2 |
bgpd: CVE-2012-1820, DoS in bgp_capability_orf()
|
|
Adam Tkac |
dd793e2 |
|
|
Adam Tkac |
dd793e2 |
An ORF (code 3) capability TLV is defined to contain exactly one
|
|
Adam Tkac |
dd793e2 |
AFI/SAFI block. Function bgp_capability_orf(), which parses ORF
|
|
Adam Tkac |
dd793e2 |
capability TLV, uses do-while cycle to call its helper function
|
|
Adam Tkac |
dd793e2 |
bgp_capability_orf_entry(), which actually processes the AFI/SAFI data
|
|
Adam Tkac |
dd793e2 |
block. The call is made at least once and repeated as long as the input
|
|
Adam Tkac |
dd793e2 |
buffer has enough data for the next call.
|
|
Adam Tkac |
dd793e2 |
|
|
Adam Tkac |
dd793e2 |
The helper function, bgp_capability_orf_entry(), uses "Number of ORFs"
|
|
Adam Tkac |
dd793e2 |
field of the provided AFI/SAFI block to verify, if it fits the input
|
|
Adam Tkac |
dd793e2 |
buffer. However, the check is made based on the total length of the ORF
|
|
Adam Tkac |
dd793e2 |
TLV regardless of the data already consumed by the previous helper
|
|
Adam Tkac |
dd793e2 |
function call(s). This way, the check condition is only valid for the
|
|
Adam Tkac |
dd793e2 |
first AFI/SAFI block inside an ORF capability TLV.
|
|
Adam Tkac |
dd793e2 |
|
|
Adam Tkac |
dd793e2 |
For the subsequent calls of the helper function, if any are made, the
|
|
Adam Tkac |
dd793e2 |
check condition may erroneously tell, that the current "Number of ORFs"
|
|
Adam Tkac |
dd793e2 |
field fits the buffer boundary, where in fact it does not. This makes it
|
|
Adam Tkac |
dd793e2 |
possible to trigger an assertion by feeding an OPEN message with a
|
|
Adam Tkac |
dd793e2 |
specially-crafted malformed ORF capability TLV.
|
|
Adam Tkac |
dd793e2 |
|
|
Adam Tkac |
dd793e2 |
This commit fixes the vulnerability by making the implementation follow
|
|
Adam Tkac |
dd793e2 |
the spec.
|
|
Adam Tkac |
dd793e2 |
--- a/bgpd/bgp_open.c
|
|
Adam Tkac |
dd793e2 |
+++ a/bgpd/bgp_open.c
|
|
Adam Tkac |
dd793e2 |
@@ -231,7 +231,7 @@ bgp_capability_orf_entry (struct peer *peer, struct capability_header *hdr)
|
|
Adam Tkac |
dd793e2 |
}
|
|
Adam Tkac |
dd793e2 |
|
|
Adam Tkac |
dd793e2 |
/* validate number field */
|
|
Adam Tkac |
dd793e2 |
- if (sizeof (struct capability_orf_entry) + (entry.num * 2) > hdr->length)
|
|
Adam Tkac |
dd793e2 |
+ if (sizeof (struct capability_orf_entry) + (entry.num * 2) != hdr->length)
|
|
Adam Tkac |
dd793e2 |
{
|
|
Adam Tkac |
dd793e2 |
zlog_info ("%s ORF Capability entry length error,"
|
|
Adam Tkac |
dd793e2 |
" Cap length %u, num %u",
|
|
Adam Tkac |
dd793e2 |
@@ -335,28 +335,6 @@ bgp_capability_orf_entry (struct peer *peer, struct capability_header *hdr)
|
|
Adam Tkac |
dd793e2 |
}
|
|
Adam Tkac |
dd793e2 |
|
|
Adam Tkac |
dd793e2 |
static int
|
|
Adam Tkac |
dd793e2 |
-bgp_capability_orf (struct peer *peer, struct capability_header *hdr)
|
|
Adam Tkac |
dd793e2 |
-{
|
|
Adam Tkac |
dd793e2 |
- struct stream *s = BGP_INPUT (peer);
|
|
Adam Tkac |
dd793e2 |
- size_t end = stream_get_getp (s) + hdr->length;
|
|
Adam Tkac |
dd793e2 |
-
|
|
Adam Tkac |
dd793e2 |
- assert (stream_get_getp(s) + sizeof(struct capability_orf_entry) <= end);
|
|
Adam Tkac |
dd793e2 |
-
|
|
Adam Tkac |
dd793e2 |
- /* We must have at least one ORF entry, as the caller has already done
|
|
Adam Tkac |
dd793e2 |
- * minimum length validation for the capability code - for ORF there must
|
|
Adam Tkac |
dd793e2 |
- * at least one ORF entry (header and unknown number of pairs of bytes).
|
|
Adam Tkac |
dd793e2 |
- */
|
|
Adam Tkac |
dd793e2 |
- do
|
|
Adam Tkac |
dd793e2 |
- {
|
|
Adam Tkac |
dd793e2 |
- if (bgp_capability_orf_entry (peer, hdr) == -1)
|
|
Adam Tkac |
dd793e2 |
- return -1;
|
|
Adam Tkac |
dd793e2 |
- }
|
|
Adam Tkac |
dd793e2 |
- while (stream_get_getp(s) + sizeof(struct capability_orf_entry) < end);
|
|
Adam Tkac |
dd793e2 |
-
|
|
Adam Tkac |
dd793e2 |
- return 0;
|
|
Adam Tkac |
dd793e2 |
-}
|
|
Adam Tkac |
dd793e2 |
-
|
|
Adam Tkac |
dd793e2 |
-static int
|
|
Adam Tkac |
dd793e2 |
bgp_capability_restart (struct peer *peer, struct capability_header *caphdr)
|
|
Adam Tkac |
dd793e2 |
{
|
|
Adam Tkac |
dd793e2 |
struct stream *s = BGP_INPUT (peer);
|
|
Adam Tkac |
dd793e2 |
@@ -573,7 +551,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int *mp_capability,
|
|
Adam Tkac |
dd793e2 |
break;
|
|
Adam Tkac |
dd793e2 |
case CAPABILITY_CODE_ORF:
|
|
Adam Tkac |
dd793e2 |
case CAPABILITY_CODE_ORF_OLD:
|
|
Adam Tkac |
dd793e2 |
- if (bgp_capability_orf (peer, &caphdr))
|
|
Adam Tkac |
dd793e2 |
+ if (bgp_capability_orf_entry (peer, &caphdr))
|
|
Adam Tkac |
dd793e2 |
return -1;
|
|
Adam Tkac |
dd793e2 |
break;
|
|
Adam Tkac |
dd793e2 |
case CAPABILITY_CODE_RESTART:
|