From 6254f4400110d1bf1708ddb505f0cd37eb36d9b5 Mon Sep 17 00:00:00 2001 From: d00f Date: Sat, 8 Aug 2026 17:13:08 +0200 Subject: [PATCH] cmd: reject out-of-range numeric arguments instead of wrapping atoi_short() accumulated into a uint16_t without checking, so "vlan 65540" wrapped to 4 and edited VLAN 4 instead of failing. atoi_byte() had the same hole with "300" landing on 44. Both now refuse the digit that would push the value past its type, before it lands. The partial result is deliberately left alone rather than zeroed. Zeroing would give the function one tidy rule, every failure leaves 0, but a caller that ignores the return would then write that 0, and 0 is not a harmless number everywhere. Set as a port MTU it stops the port taking frames: I put 0 on a live 2.5G LAG member and its LACPDU receives moved by 4 in fifteen seconds against 21 on the sibling port, with the partner going expired. Putting the size back recovered both. A wrong number does less damage than that, and the real fix belongs in the callers that ignore the return anyway. The test sits inside the loop rather than after it, so no wider accumulator is needed and the parser stays off the internal RAM budget. 056a30a on the branch in #303 fixes atoi_byte a different way, by widening the accumulator. Whichever lands first, the other hunk should go. --- cmd_parser.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd_parser.c b/cmd_parser.c index 4e8107e..88e39f2 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -191,8 +191,11 @@ uint8_t atoi_byte(__xdata uint8_t *out, uint8_t idx) uint8_t num = 0; while (isnumber(cmd_buffer[idx])) { + uint8_t val = cmd_buffer[idx] - '0'; err = 0; - num = (num * 10) + cmd_buffer[idx] - '0'; + if (num > 25 || (num == 25 && val > 5)) + return 1; + num = (num * 10) + val; idx++; } @@ -209,6 +212,8 @@ uint8_t atoi_short(__xdata uint16_t *vlan, uint8_t idx) while (isnumber(cmd_buffer[idx])) { err = 0; uint8_t val = cmd_buffer[idx] - '0'; + if (*vlan > 6553 || (*vlan == 6553 && val > 5)) + return 1; *vlan = (*vlan * 10) + val; idx++; }