From 862260de172c1c445e24b83b848b21cefe3ca7f3 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 01:19:09 -0700 Subject: [PATCH] Reject AGC intervals that would wrap or silently disable resets --- src/helpers/CommonCLI.cpp | 11 +++++++--- test/fixtures/cli_settings/main.cpp | 33 +++++++++++++++++++++++++++++ test/test_cli_settings_contract.py | 10 ++++++--- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 999b496c2..323bb026b 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -4265,9 +4265,14 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep strcpy(reply, "OK"); } } else if (memcmp(config, "agc.reset.interval ", 19) == 0) { - _prefs->agc_reset_interval = atoi(&config[19]) / 4; - savePrefs(); - sprintf(reply, "OK - interval rounded to %d", ((uint32_t) _prefs->agc_reset_interval) * 4); + uint32_t seconds; + if (!mesh::cli::parseUnsignedIntegerStrict(&config[19], seconds) || seconds > 1020) { + strcpy(reply, "Error: AGC interval must be 0-1020 seconds"); + } else { + _prefs->agc_reset_interval = seconds / 4; + savePrefs(); + sprintf(reply, "OK - interval rounded to %d", ((uint32_t) _prefs->agc_reset_interval) * 4); + } } else if (memcmp(config, "multi.acks ", 11) == 0) { _prefs->multi_acks = atoi(&config[11]); savePrefs(); diff --git a/test/fixtures/cli_settings/main.cpp b/test/fixtures/cli_settings/main.cpp index a03130ab4..46cea29f8 100644 --- a/test/fixtures/cli_settings/main.cpp +++ b/test/fixtures/cli_settings/main.cpp @@ -134,6 +134,39 @@ int main() { assert(!strncmp(reply+2,state,strlen(state))); } } + // The saved byte represents four-second units. Reject malformed/out-of- + // range input before it can wrap (1024 used to silently disable AGC). + for (unsigned seconds=0; seconds<=1020; ++seconds) { + const unsigned saves=cli.callbacks.saves; + snprintf(text,sizeof(text),"set agc.reset.interval %u",seconds); + cli.call(sender,text,reply); + assert(!strncmp(reply,"OK",2) && cli.callbacks.saves==saves+1); + assert(cli.prefs.agc_reset_interval==seconds/4); + cli.call(sender,"get agc.reset.interval",reply); + unsigned actual=9999; + assert(sscanf(reply,"> %u",&actual)==1 && actual==(seconds/4)*4); + assert(cli.callbacks.saves==saves+1); + } + for (const char* value : {"1024", "1021", "1023", "-1", "-4", "", + "garbage", "19tail", "1.5", "0x10", "+", "--4", + "4294967295", "4294967296", "2147483648", + "99999999999999999999999999999999999999"}) { + const unsigned saves=cli.callbacks.saves; + const uint8_t previous=cli.prefs.agc_reset_interval; + snprintf(text,sizeof(text),"set agc.reset.interval %s",value); + cli.call(sender,text,reply); + if (strncmp(reply,"Error",5)) + fprintf(stderr,"invalid AGC interval %s accepted: %s\n",value,reply); + assert(!strncmp(reply,"Error",5)); + assert(cli.prefs.agc_reset_interval==previous && cli.callbacks.saves==saves); + } + for (const char* value : {"+19", " 19 ", "\t19\t", "00019"}) { + const unsigned saves=cli.callbacks.saves; + snprintf(text,sizeof(text),"set agc.reset.interval %s",value); + cli.call(sender,text,reply); + assert(!strncmp(reply,"OK",2) && cli.callbacks.saves==saves+1); + assert(cli.prefs.agc_reset_interval==4); + } unsigned saves=cli.callbacks.saves; cli.call(sender,"set radio 916,125,8,6",reply); assert(!strncmp(reply,"OK",2) && cli.callbacks.saves==saves+1); diff --git a/test/test_cli_settings_contract.py b/test/test_cli_settings_contract.py index 127655bdf..e1782f67f 100644 --- a/test/test_cli_settings_contract.py +++ b/test/test_cli_settings_contract.py @@ -8,6 +8,7 @@ from pathlib import Path import re import shutil import subprocess +import sys import tempfile import unittest from test_replay_reset_integration import extract_braced @@ -259,9 +260,12 @@ class CLISettingsContractTest(unittest.TestCase): cpp.write_text(fixture, encoding='utf-8') for macros in ([], ['-DUSE_LR2021=1']): with self.subTest(macros=macros): - subprocess.run([compiler, '-std=c++17', '-Wall', '-Wextra', '-Werror', *macros, - '-I', str(ROOT/'src'), str(cpp), '-o', str(exe)], check=True) - subprocess.run([str(exe)], check=True) + sanitizers = (['-fsanitize=address,undefined', '-fno-sanitize-recover=all', + '-fno-pie', '-no-pie'] if sys.platform.startswith('linux') else []) + subprocess.run([compiler, '-std=c++17', '-Wall', '-Wextra', '-Werror', + *sanitizers, *macros, '-I', str(ROOT/'src'), str(cpp), + '-o', str(exe)], check=True, timeout=60) + subprocess.run([str(exe)], check=True, timeout=15) if __name__ == '__main__':