From 1b514b5ea8134e14822b63d5d91c2f3a01359eb7 Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:38:37 +0200 Subject: [PATCH] vlan: bound the name table before writing into it A VLAN name is copied into vlan_names with nothing checking vlan_ptr against VLAN_NAMES_SIZE. The only limit is the 128 byte command line, so enough named VLANs walk straight off the end of a 1 kB table. What sits immediately behind it makes this worse than a plain overflow: at 0x048A comes vlan_ptr itself, the index driving the write, then port_names and cmd_buffer. Once the index is corrupted the following writes land wherever it now points, and the configuration is replayed at boot, so a config file that overflows does it again on every start. Measure the name first and refuse it if the three id digits, the name, its separating space and the terminator would not fit. The check goes before vlan_name_remove() so a name that does not fit cannot take the existing one with it, and the VLAN still gets its ports: dropping the whole line would lose port membership during a config replay, which is worse than losing a name. The copy walks a pointer instead of indexing vlan_names[vlan_ptr++] each time, and the word offset comes out of cmd_words_b once. Writing the bound as VLAN_NAMES_SIZE - 5 folds the constant at compile time instead of adding it on every call. BANK2 ends up 25 bytes smaller than it was before the check existed. --- cmd_parser.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 4fc2b0cf..2b1e822e 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -504,19 +504,29 @@ void parse_vlan(void) } uint8_t w = 2; - if (cmd_words_len > w && isletter(cmd_buffer[cmd_words_b[w]])) { + uint8_t pos = cmd_words_b[w]; + if (cmd_words_len > w && isletter(cmd_buffer[pos])) { uint8_t i = 0; - vlan_name_remove(vlan_settings.vlan); - vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; - vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; - vlan_names[vlan_ptr++] = hex[vlan_settings.vlan & 0xf]; - while(!cmd_is_space_or_nul(cmd_words_b[w] + i)) { - write_char(cmd_buffer[cmd_words_b[w] + i]); - vlan_names[vlan_ptr++] = cmd_buffer[cmd_words_b[w] + i++]; + while (!cmd_is_space_or_nul(pos + i)) + i++; + if (vlan_ptr + i > (VLAN_NAMES_SIZE - 5)) { + print_string("VLAN name table full, name ignored\n"); + } else { + vlan_name_remove(vlan_settings.vlan); + __xdata uint8_t *vlan_str = &vlan_names[vlan_ptr]; + *vlan_str++ = hex[(vlan_settings.vlan >> 8) & 0xf]; + *vlan_str++ = hex[(vlan_settings.vlan >> 4) & 0xf] ; + *vlan_str++ = hex[vlan_settings.vlan & 0xf]; + while(!cmd_is_space_or_nul(pos)) { + uint8_t c = cmd_buffer[pos++]; + write_char(c); + *vlan_str++ = c; + } + *vlan_str++ = ' '; *vlan_str = NUL; + vlan_ptr = vlan_str - vlan_names; + print_string("<\n"); } - vlan_names[vlan_ptr++] = ' '; vlan_names[vlan_ptr] = NUL; w++; - print_string("<\n"); } uint8_t ret;