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

Fix parser #2875

Merged
merged 4 commits into from
Feb 7, 2025
Merged

Fix parser #2875

merged 4 commits into from
Feb 7, 2025

Conversation

fujita
Copy link
Member

@fujita fujita commented Feb 7, 2025

No description provided.

ivg added 4 commits February 7, 2025 10:10
 1 func (m *BGP4MPHeader) decodeFromBytes(data []byte) ([]byte, error) {
 2     if m.isAS4 && len(data) < 8 {
 3         return nil, errors.New("not all BGP4MPMessageAS4 bytes available")
 4     } else if !m.isAS4 && len(data) < 4 {
 5         return nil, errors.New("not all BGP4MPMessageAS bytes available")
 6     }
 7
 8     if m.isAS4 {
 9         m.PeerAS = binary.BigEndian.Uint32(data[:4])
10         m.LocalAS = binary.BigEndian.Uint32(data[4:8])
11         data = data[8:]
12     } else {
13         m.PeerAS = uint32(binary.BigEndian.Uint16(data[:2]))
14         m.LocalAS = uint32(binary.BigEndian.Uint16(data[2:4]))
15         data = data[4:]
16     }
17     m.InterfaceIndex = binary.BigEndian.Uint16(data[:2])
18     m.AddressFamily = binary.BigEndian.Uint16(data[2:4])
19     switch m.AddressFamily {
20     case bgp.AFI_IP:
21         m.PeerIpAddress = net.IP(data[4:8]).To4()
22         m.LocalIpAddress = net.IP(data[8:12]).To4()
23         data = data[12:]
24     case bgp.AFI_IP6:
25         m.PeerIpAddress = net.IP(data[4:20])
26         m.LocalIpAddress = net.IP(data[20:36])
27         data = data[36:]
28     default:
29         return nil, fmt.Errorf("unsupported address family: %d",
m.AddressFamily)
30     }
31     return data, nil
32 }

The check  at lines 2-6 is sufficient only to safely extract `PeerAS`
and `LocalAS`, after that slice is rebound to the leftover bytes,
i.e., the length of the slice is not 8 or 4 bytes less, depending on
the it is AS4 or not. That means that it could be empty, therefore any
line beyond 16 is vulnerable. E.g., we need to check for at least 4
bytes for lines 17 and 18, and then depending on the address family,
for 12 or 36 bytes.
case EC_SUBTYPE_FLOWSPEC_REDIRECT_IP6:
   ipv6 := net.IP(data[2:18]).String()
   localAdmin := binary.BigEndian.Uint16(data[18:20])
   return NewRedirectIPv6AddressSpecificExtended(ipv6, localAdmin), nil

Note that the `data` length is only checked for being at least 8
bytes, so any message with the given subtype but less than 20 bytes
will crash the application.
…length

func (c *CapSoftwareVersion) DecodeFromBytes(data []byte) error {
c.DefaultParameterCapability.DecodeFromBytes(data)
data = data[2:]
if len(data) < 2 {
return NewMessageError(BGP_ERROR_OPEN_MESSAGE_ERROR, BGP_ERROR_SUB_UNSUPPORTED_CAPABILITY, nil, "Not all CapabilitySoftwareVersion bytes allowed")
}
softwareVersionLen := uint8(data[0])
if len(data[1:]) < int(softwareVersionLen) || softwareVersionLen > 64 {
return NewMessageError(BGP_ERROR_OPEN_MESSAGE_ERROR, BGP_ERROR_SUB_UNSUPPORTED_CAPABILITY, nil, "invalid length of software version capablity")
}
c.SoftwareVersionLen = softwareVersionLen
c.SoftwareVersion = string(data[1:c.SoftwareVersionLen]) // ivg: note the crash is here
return nil
}

Notice that `softwareVersionLen` is not checked for `0`, so
`data[1:c.SoftwareVersionLen]` becomes `data[1:0]`, which leads to a
runtime panic.
@fujita fujita merged commit 08a001e into osrg:master Feb 7, 2025
39 checks passed
@fujita fujita deleted the fix-parser branch February 7, 2025 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants