Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: the length check of bgp_capability_llgr is not correct #13098

Closed
2 tasks
melissa-cjt opened this issue Mar 24, 2023 · 9 comments
Closed
2 tasks

bgpd: the length check of bgp_capability_llgr is not correct #13098

melissa-cjt opened this issue Mar 24, 2023 · 9 comments
Assignees
Labels

Comments

@melissa-cjt
Copy link


Describe the bug

  • Did you check if this is a duplicate issue?
  • Did you test it on the latest FRRouting/frr master branch?

Hello, I have find a bug in bgp_capability_llgr that it length check is wrong. In the draft document it has 7 bytes as shown in the following figure.

The capability value consists of zero or more tuples <AFI, SAFI,
Flags, Long-lived Stale Time> as follows:

  +--------------------------------------------------+
  | Address Family Identifier (16 bits)              |
  +--------------------------------------------------+
  | Subsequent Address Family Identifier (8 bits)    |
  +--------------------------------------------------+
  | Flags for Address Family (8 bits)                |
  +--------------------------------------------------+
  | Long-lived Stale Time (24 bits)                  |
  +--------------------------------------------------+

While, in the code it only check 4 bytes.

while (stream_get_getp(s) + 4 <= end) {
	afi_t afi;
	safi_t safi;
	iana_afi_t pkt_afi = stream_getw(s);
	iana_safi_t pkt_safi = stream_getc(s);
	uint8_t flags = stream_getc(s);
	uint32_t stale_time = stream_get3(s);

   }

To Reproduce
If I construct a packet only has 6 bytes of the llgr, the frrrouting will crash.

BGP: in thread bgp_process_packet scheduled from bgpd/bgp_io.c:269 bgp_process_reads()
core_handler: showing active allocations in memory group libfrr
core_handler: memstats: Buffer : 2 * 24
core_handler: memstats: Host config : 8 * (variably sized)
core_handler: memstats: Command Tokens : 12082 * 72
core_handler: memstats: Command Token Text : 8746 * (variably sized)
core_handler: memstats: Command Token Help : 8746 * (variably sized)
core_handler: memstats: Command Argument Name : 2052 * (variably sized)
core_handler: memstats: RCU thread : 2 * 128
core_handler: memstats: FRR POSIX Thread : 4 * (variably sized)
core_handler: memstats: POSIX sync primitives : 4 * (variably sized)
core_handler: memstats: Graph : 40 * 8
core_handler: memstats: Graph Node : 14266 * 32
core_handler: memstats: Hash : 573 * (variably sized)
core_handler: memstats: Hash Bucket : 2340 * 32
core_handler: memstats: Hash Index : 287 * (variably sized)
core_handler: memstats: Link List : 36 * 40
core_handler: memstats: Link Node : 334 * 24
core_handler: memstats: Temporary memory : 15 * (variably sized)
core_handler: memstats: Bitfield memory : 2 * (variably sized)
core_handler: memstats: Northbound Node : 240 * 1192
core_handler: memstats: Northbound Configuration : 2 * 16
core_handler: memstats: Privilege information : 3 * (variably sized)
core_handler: memstats: Ring buffer : 6 * (variably sized)
core_handler: memstats: Skip List : 2 * 56
core_handler: memstats: Skip Node : 2 * 160
core_handler: memstats: Skiplist Counters : 2 * 68
core_handler: memstats: Socket union : 2 * 112
core_handler: memstats: Stream : 12 * (variably sized)
core_handler: memstats: Stream FIFO : 6 * 64
core_handler: memstats: Route table : 100 * 56
core_handler: memstats: Thread : 15 * 160
core_handler: memstats: Thread master : 12 * (variably sized)
core_handler: memstats: Thread Poll Info : 6 * 8388608
core_handler: memstats: Thread stats : 18 * 96
core_handler: memstats: Typed-hash bucket : 5 * (variably sized)
core_handler: memstats: Typed-heap array : 1 * 576
core_handler: memstats: Vector : 28613 * 24
core_handler: memstats: Vector index : 28613 * (variably sized)
core_handler: memstats: VRF : 1 * 216
core_handler: memstats: VTY server : 3 * 32
core_handler: memstats: Work queue : 3 * 152
core_handler: memstats: Work queue name string : 3 * (variably sized)
core_handler: memstats: YANG module : 5 * 48
core_handler: memstats: Zclient : 2 * 3144
core_handler: memstats: Redistribution instance IDs : 6 * 2
core_handler: memstats: log thread-local buffer : 2 * 24608
core_handler: showing active allocations in memory group logging subsystem
core_handler: memstats: log file target : 2 * 88
core_handler: memstats: log file name : 1 * 14
core_handler: showing active allocations in memory group bgpd
core_handler: memstats: BGP instance : 2 * (variably sized)
core_handler: memstats: BGP listen socket details : 2 * 144
core_handler: memstats: BGP peer : 3 * 740824
core_handler: memstats: BGP peer hostname : 4 * (variably sized)
core_handler: memstats: BGP peer af : 2 * 80
core_handler: memstats: BGP attribute : 1 * 312
core_handler: memstats: BGP aspath : 1 * 40
core_handler: memstats: BGP aspath str : 1 * 1
core_handler: memstats: BGP table : 87 * 56
core_handler: memstats: BGP node : 2 * 192
core_handler: memstats: BGP route : 1 * 112
core_handler: memstats: BGP static : 1 * 144
core_handler: memstats: BGP synchronise : 63 * 72
core_handler: memstats: community-list handler : 1 * 120
core_handler: memstats: BGP nexthop : 1 * 184
core_handler: memstats: BGP EVPN MH Information : 1 * 56
core_handler: memstats: BGP PBR Context : 1 * 32
core_handler: memstats: BGP EVPN instance information : 1 * 56
core_handler: showing active allocations in memory group rfapi
core_handler: memstats: NVE Configuration : 1 * 2984
core_handler: memstats: RFAPI Generic : 1 * 296
core_handler: memstats: RFAPI Import Table : 1 * 208
Aborted (core dumped)

Expected behavior

Screenshots

Versions

  • OS Version:
  • Kernel:
  • FRR Version:

Additional context

@melissa-cjt melissa-cjt added the triage Needs further investigation label Mar 24, 2023
@ton31337 ton31337 added the bgp label Mar 24, 2023
@ton31337
Copy link
Member

Can you share the script to test it?

@ton31337 ton31337 self-assigned this Mar 24, 2023
@melissa-cjt
Copy link
Author

Sure, blow is my poc.
poc-llgr.zip

@ton31337 ton31337 removed the triage Needs further investigation label Mar 24, 2023
@ton31337
Copy link
Member

Thanks, fixed.

@abergmann
Copy link

CVE-2023-31489 was assigned to this issue.

@abergmann
Copy link

@ton31337 could you reference a commit or PR, please?

@ton31337
Copy link
Member

b1d33ec

@zzxgzgz
Copy link

zzxgzgz commented May 22, 2023

Sure, blow is my poc. poc-llgr.zip

Hi @melissa-cjt ,

Could you please share with me that how do you run the script? I tried to run it, but I didn't see my bgpd crash. The command I used is:

python ./poc-llgr.py 123 127.0.0.1

Thank you for your help.

@risicle
Copy link

risicle commented May 29, 2023

It would be nice to have a release including the fix for this and #13099 seeing as NVD has marked them as medium & high.

Edit: It looks like this one is addressed in 8.5.1 via 5ce4019

@eqvinox
Copy link
Contributor

eqvinox commented Jun 13, 2023

0d50691 for 8.4, 32 commits after 8.4.3, need 8.4.4 release.

halstead pushed a commit to openembedded/meta-openembedded that referenced this issue Jul 6, 2023
An issue found in Frrouting bgpd v.8.4.2 allows a remote attacker to
cause a denial of service via the bgp_capability_llgr() function.

References:
https://nvd.nist.gov/vuln/detail/CVE-2023-31489
FRRouting/frr#13098

Signed-off-by: Narpat Mali <narpat.mali@windriver.com>
[Refactored to get it to apply]
Signed-off-by: Armin Kuster <akuster808@gmail.com>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this issue Jul 24, 2023
Source: meta-openembedded
MR: 126092
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: 0070827
Description:

An issue found in Frrouting bgpd v.8.4.2 allows a remote attacker to
cause a denial of service via the bgp_capability_llgr() function.

References:
https://nvd.nist.gov/vuln/detail/CVE-2023-31489
FRRouting/frr#13098

Signed-off-by: Narpat Mali <narpat.mali@windriver.com>
[Refactored to get it to apply]
Signed-off-by: Armin Kuster <akuster808@gmail.com>
Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants