bgpd: fix strlcat/strlcpy size parameter in NOTIFICATION send path#22279
bgpd: fix strlcat/strlcpy size parameter in NOTIFICATION send path#22279guoguojia2021 wants to merge 1 commit into
Conversation
Greptile SummaryFixes incorrect buffer size arguments in
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to two size arguments in a debug-only code block, with no effect on packet construction or protocol behavior. Both changed lines are inside the /* For debug */ block and only affect how hex bytes are printed into a local string before being passed to bgp_notify_print. The buffer allocation size is unchanged and correctly sized at length * 3; the fix simply passes that same value to strlcat/strlcpy so the full payload is rendered. The receive path already used this exact pattern, confirming the intended approach. No data path, memory ownership, or protocol logic is touched. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[bgp_notify_send_internal called] --> B{datalen > 0 and data != NULL?}
B -- No --> E[bgp_notify.data = NULL]
B -- Yes --> C[Allocate buffer: XMALLOC datalen * 3]
C --> D[Loop i = 0..datalen-1]
D --> F{first byte?}
F -- Yes --> G[strlcpy with size datalen * 3]
F -- No --> H[strlcat with size datalen * 3]
G --> D
H --> D
E --> I[bgp_notify_print]
D --> I
I --> J[XFREE bgp_notify.data]
Reviews (1): Last reviewed commit: "bgpd: fix strlcat/strlcpy size parameter..." | Re-trigger Greptile |
|
|
||
| strlcat(bgp_notify.data, c, | ||
| bgp_notify.length); | ||
| bgp_notify.length * 3); |
There was a problem hiding this comment.
let's just set the correct size of bgp_notify.length on line 1011 instead of having to multiple *3 all over the place.
There was a problem hiding this comment.
Thanks for the suggestion. Updated!
677fb54 to
5ccd33b
Compare
| bgp_notify.subcode = sub_code; | ||
| bgp_notify.data = NULL; | ||
| bgp_notify.length = datalen; | ||
| notify_data_size = bgp_notify.length * 3; |
There was a problem hiding this comment.
Why is bgp_notify.length set to the correct value here and just use that?
There was a problem hiding this comment.
Thanks for the suggestion. Updated to set bgp_notify.length = datalen * 3 directly.
In bgp_notify_send_internal(), the hex-formatted notification data buffer is allocated with size bgp_notify.length * 3, since each byte is formatted as " %02x" (3 characters). However, the strlcat() and strlcpy() calls were incorrectly passed bgp_notify.length as the buffer size limit instead of bgp_notify.length * 3. This caused severe truncation of the debug output string. For example, a 30-byte notification data would allocate a 90-byte buffer, but strlcpy would only allow writing 30 bytes, displaying roughly the first 10 bytes of hex values and silently discarding the rest. The receive path (bgp_notify_receive) already correctly uses bgp_notify.length * 3 as the size limit for the same formatting logic. This fix makes the send path consistent with the receive path. While strlcat/strlcpy are safe functions that truncate rather than overflow, the truncated output made NOTIFICATION debugging unreliable, potentially hiding critical protocol error details from operators during fault diagnosis. Signed-off-by: guozhongfeng <guozhongfeng.gzf@alibaba-inc.com>
5ccd33b to
7f3c90e
Compare
In bgp_notify_send_internal(), the hex-formatted notification data buffer is allocated with size bgp_notify.length * 3, since each byte is formatted as " %02x" (3 characters). However, the strlcat() and strlcpy() calls were incorrectly passed bgp_notify.length as the buffer size limit instead of bgp_notify.length * 3.
This caused severe truncation of the debug output string. For example, a 30-byte notification data would allocate a 90-byte buffer, but strlcpy would only allow writing 30 bytes, displaying roughly the first 10 bytes of hex values and silently discarding the rest.
The receive path (bgp_notify_receive) already correctly uses bgp_notify.length * 3 as the size limit for the same formatting logic. This fix makes the send path consistent with the receive path.
While strlcat/strlcpy are safe functions that truncate rather than overflow, the truncated output made NOTIFICATION debugging unreliable, potentially hiding critical protocol error details from operators during fault diagnosis.