zebra: Validate SRv6 locator name length in VTY commands#21905
zebra: Validate SRv6 locator name length in VTY commands#21905cscarpitta wants to merge 1 commit into
Conversation
Currently, when a user configures an SRv6 locator with a name exceeding SRV6_LOCNAME_SIZE (255 characters), the 'locator' command passes the full input to srv6_locator_alloc(), which silently truncates the name via strlcpy() into the fixed-size name buffer. This results in the allocated locator having a different name than the user requested Add explicit input validation to reject names exceeding the maximum length in both 'locator' (create) and 'no locator' (delete) VTY commands, ensuring the stored locator name matches user intent. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Greptile SummaryThis PR adds explicit input validation to reject SRv6 locator names that meet or exceed
Confidence Score: 4/5The change is narrowly scoped to input validation in two VTY command handlers and does not touch any data structures, allocation logic, or client notification paths. The boundary check (>= SRV6_LOCNAME_SIZE) is correct for a 256-byte buffer and the error message accurately reports 255 as the maximum. The only notes are two %u format specifiers used with an int expression that could produce -Wformat warnings under strict compiler settings. No files require special attention beyond the two format-specifier suggestions in zebra/zebra_srv6_vty.c. Important Files Changed
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
zebra/zebra_srv6_vty.c:804-808
The `%u` format specifier expects an `unsigned int`, but `SRV6_LOCNAME_SIZE - 1` is an `int` (the macro expands to the integer literal `256`). While benign for this value, it can trigger a `-Wformat` compiler warning. Using `%d` matches the actual type.
```suggestion
if (strlen(argv[1]->arg) >= SRV6_LOCNAME_SIZE) {
vty_out(vty, "%% Invalid locator name: maximum length is %d characters\n",
SRV6_LOCNAME_SIZE - 1);
return CMD_WARNING_CONFIG_FAILED;
}
```
### Issue 2 of 2
zebra/zebra_srv6_vty.c:838-842
Same `%u` vs `int` mismatch as in the `locator` command above — use `%d` to match the type of `SRV6_LOCNAME_SIZE - 1`.
```suggestion
if (strlen(argv[2]->arg) >= SRV6_LOCNAME_SIZE) {
vty_out(vty, "%% Invalid locator name: maximum length is %d characters\n",
SRV6_LOCNAME_SIZE - 1);
return CMD_WARNING_CONFIG_FAILED;
}
```
Reviews (1): Last reviewed commit: "zebra: Validate SRv6 locator name length..." | Re-trigger Greptile |
| if (strlen(argv[1]->arg) >= SRV6_LOCNAME_SIZE) { | ||
| vty_out(vty, "%% Invalid locator name: maximum length is %u characters\n", | ||
| SRV6_LOCNAME_SIZE - 1); | ||
| return CMD_WARNING_CONFIG_FAILED; | ||
| } |
There was a problem hiding this comment.
The
%u format specifier expects an unsigned int, but SRV6_LOCNAME_SIZE - 1 is an int (the macro expands to the integer literal 256). While benign for this value, it can trigger a -Wformat compiler warning. Using %d matches the actual type.
| if (strlen(argv[1]->arg) >= SRV6_LOCNAME_SIZE) { | |
| vty_out(vty, "%% Invalid locator name: maximum length is %u characters\n", | |
| SRV6_LOCNAME_SIZE - 1); | |
| return CMD_WARNING_CONFIG_FAILED; | |
| } | |
| if (strlen(argv[1]->arg) >= SRV6_LOCNAME_SIZE) { | |
| vty_out(vty, "%% Invalid locator name: maximum length is %d characters\n", | |
| SRV6_LOCNAME_SIZE - 1); | |
| return CMD_WARNING_CONFIG_FAILED; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: zebra/zebra_srv6_vty.c
Line: 804-808
Comment:
The `%u` format specifier expects an `unsigned int`, but `SRV6_LOCNAME_SIZE - 1` is an `int` (the macro expands to the integer literal `256`). While benign for this value, it can trigger a `-Wformat` compiler warning. Using `%d` matches the actual type.
```suggestion
if (strlen(argv[1]->arg) >= SRV6_LOCNAME_SIZE) {
vty_out(vty, "%% Invalid locator name: maximum length is %d characters\n",
SRV6_LOCNAME_SIZE - 1);
return CMD_WARNING_CONFIG_FAILED;
}
```
How can I resolve this? If you propose a fix, please make it concise.| if (strlen(argv[2]->arg) >= SRV6_LOCNAME_SIZE) { | ||
| vty_out(vty, "%% Invalid locator name: maximum length is %u characters\n", | ||
| SRV6_LOCNAME_SIZE - 1); | ||
| return CMD_WARNING_CONFIG_FAILED; | ||
| } |
There was a problem hiding this comment.
Same
%u vs int mismatch as in the locator command above — use %d to match the type of SRV6_LOCNAME_SIZE - 1.
| if (strlen(argv[2]->arg) >= SRV6_LOCNAME_SIZE) { | |
| vty_out(vty, "%% Invalid locator name: maximum length is %u characters\n", | |
| SRV6_LOCNAME_SIZE - 1); | |
| return CMD_WARNING_CONFIG_FAILED; | |
| } | |
| if (strlen(argv[2]->arg) >= SRV6_LOCNAME_SIZE) { | |
| vty_out(vty, "%% Invalid locator name: maximum length is %d characters\n", | |
| SRV6_LOCNAME_SIZE - 1); | |
| return CMD_WARNING_CONFIG_FAILED; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: zebra/zebra_srv6_vty.c
Line: 838-842
Comment:
Same `%u` vs `int` mismatch as in the `locator` command above — use `%d` to match the type of `SRV6_LOCNAME_SIZE - 1`.
```suggestion
if (strlen(argv[2]->arg) >= SRV6_LOCNAME_SIZE) {
vty_out(vty, "%% Invalid locator name: maximum length is %d characters\n",
SRV6_LOCNAME_SIZE - 1);
return CMD_WARNING_CONFIG_FAILED;
}
```
How can I resolve this? If you propose a fix, please make it concise.|
shouldn't this require yang model changes? |
Currently, when a user configures an SRv6 locator with a name exceeding
SRV6_LOCNAME_SIZE(255 characters), the 'locator' command passes the full input tosrv6_locator_alloc(), which silently truncates the name via strlcpy() into the fixed-size name buffer. This results in the allocated locator having a different name than the user requestedAdd explicit input validation to reject names exceeding the maximum length in both
locator(create) andno locator(delete) VTY commands, ensuring the stored locator name matches user intent.