pmrfc3164: accept IPv6 literals as HOSTNAME tokens#158
pmrfc3164: accept IPv6 literals as HOSTNAME tokens#158
Conversation
Summary of ChangesHello @rgerhards, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where the RFC3164 parser failed to recognize IPv6 address literals in the HOSTNAME field, leading to incorrect parsing and potentially corrupted syslog headers. By integrating robust IPv6 hostname detection and validation, the change ensures that syslog messages from IPv6-only environments are processed accurately, preserving the integrity and correct formatting of forwarded messages. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces support for parsing IPv6 address literals in the HOSTNAME field of RFC3164 messages, which is a valuable enhancement for handling modern network configurations. The new isIPv6HostnameToken helper function encapsulates the IPv6 validation logic well. However, a potential buffer overflow issue was identified in the isIPv6HostnameToken function related to the size checks for the addr_buf buffer, which should be addressed to ensure memory safety.
tools/pmrfc3164.c
Outdated
| const uchar* addr_start = token; | ||
| size_t addr_len = token_len; | ||
|
|
||
| if (token_len == 0 || token_len >= sizeof(addr_buf)) { |
There was a problem hiding this comment.
The condition token_len >= sizeof(addr_buf) might lead to a buffer overflow. sizeof(addr_buf) is INET6_ADDRSTRLEN (46). If token_len is exactly 46, the subsequent addr_buf[addr_len] = '\0' on line 89 would write one byte past the end of the addr_buf array. The check should reserve space for the null terminator.
Consider changing this to token_len >= sizeof(addr_buf) - 1.
if (token_len == 0 || token_len >= sizeof(addr_buf) - 1) {
tools/pmrfc3164.c
Outdated
| } | ||
| addr_start = token + 1; | ||
| addr_len = token_len - 2; | ||
| if (addr_len == 0 || addr_len >= sizeof(addr_buf)) { |
There was a problem hiding this comment.
Similar to the previous comment, this condition addr_len >= sizeof(addr_buf) could also lead to a buffer overflow. If addr_len is exactly INET6_ADDRSTRLEN (46), the null terminator on line 89 would write out of bounds. This check should also reserve space for the null terminator.
Consider changing this to addr_len >= sizeof(addr_buf) - 1.
if (addr_len == 0 || addr_len >= sizeof(addr_buf) - 1) {
Non-tech: IPv6 support in rfc3164 parsing so legacy pipelines keep headers intact when devices send IPv6 addresses instead of DNS names. Impact: user-visible bugfix; RFC3164 forwarding stays well-formed for IPv6-only sources. Before: an IPv6 literal in HOSTNAME broke parsing and misaligned the forwarded header. After: valid IPv6 literals (optionally bracketed) are accepted as HOSTNAME; TAG parsing remains in sync. Technical: when normal hostname recognition fails, the parser scans the next token up to space. If it contains a colon and validates via inet_pton(AF_INET6), the token becomes HOSTNAME. Bracketed tokens are accepted only when permit.squarebracketsinhostname is on. The parse pointer advances past the token and any trailing space to preserve the RFC3164 flow. Hostname size limits (CONF_HOSTNAME_MAXSIZE) remain. A regression test (pmrfc3164-ipv6-hostname.sh) exercises both forms and is added to tests/Makefile.am. Fixes: rsyslog#3012 With the help of AI agents: Google Jules
rgerhards
force-pushed
the
codex/analyze-github-issue-#3012
branch
from
3f79004 to
a08d7be
Compare
1 participant
Motivation
Description
#include <arpa/inet.h>and a helperisIPv6HostnameTokenthat validates a candidate token as an IPv6 address usinginet_ptonand honors thepermit.squarebracketsinhostnameoption.tools/pmrfc3164.cto, on normal-hostname rejection, scan the next token up to the next space and, if it contains a colon, validate and accept it as an IPv6 HOSTNAME token and advance the parse pointer accordingly.CONF_HOSTNAME_MAXSIZE).Testing
./devtools/format-code.sh, which completed successfully.Codex Task