Fix missing index check for Tokenizer_GetArg functions (#2058)

MaxineMuster · web-flow · commit c1394cd6530e · 2026-04-04T13:38:31.000+02:00
* Fix missing "safeguard" for invalid iargument index for
Tokenizer_GetArgIntegerRange()      need to agree an return value in this cas. I choosed range_Min
Tokenizer_GetArgInteger()
Tokenizer_GetArgFloat()

Added check for negative index to all TokenizerGetXX functions

Added selftest for these cases to selftest_tokenizer.c

Tried to fix selftest_tasmota.c - it relayed on the previous behavior to return a value from a prevoius "TokenizeString()" call

Examlpe of what was changed:

-       SIM_SendFakeMQTTAndRunSimFrame_CMND("CT", "");
+       SIM_SendFakeMQTTAndRunSimFrame_CMND("CT", "153");

(later 153 is used for "assert" in
        SELFTEST_ASSERT_JSON_VALUE_INTEGER(0, "CT", 153);
)

* revert modifications to tasmota selftests.
Safeguarding command in src/cmnds/cmd_newLEDDriver.c (reject calls with insufficient number of arguments)
Make "0" default "addMode" for "add_dimmer()"

* Add some more checks. Hopefully it's complete now
diff --git a/src/cmnds/cmd_newLEDDriver.c b/src/cmnds/cmd_newLEDDriver.c
@@ -912,15 +912,17 @@ static commandResult_t temperature(const void *context, const char *cmd, const c
 	int tmp;
 	//if (!wal_strnicmp(cmd, "POWERALL", 8)){
 
-        ADDLOG_DEBUG(LOG_FEATURE_CMD, " temperature (%s) received with args %s",cmd,args);
-
-		Tokenizer_TokenizeString(args, 0);
+	Tokenizer_TokenizeString(args, 0);
 
-		tmp = Tokenizer_GetArgInteger(0);
+        ADDLOG_DEBUG(LOG_FEATURE_CMD, " temperature (%s) received with args %s",cmd,args);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
-		LED_SetTemperature(tmp, 1);
 
-		return CMD_RES_OK;
+	tmp = Tokenizer_GetArgInteger(0);
+	LED_SetTemperature(tmp, 1);
+	return CMD_RES_OK;
 	//}
 	//return 0;
 }
@@ -994,6 +996,9 @@ static commandResult_t enableAll(const void *context, const char *cmd, const cha
         ADDLOG_DEBUG(LOG_FEATURE_CMD, " enableAll (%s) received with args %s",cmd,args);
 
 		Tokenizer_TokenizeString(args, 0);
+		if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {
+			return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+		}
 
 		a = Tokenizer_GetArg(0);
 		if (a && !stricmp(a, "toggle")) {
@@ -1178,6 +1183,9 @@ static commandResult_t add_temperature(const void *context, const char *cmd, con
 	int bWrapAroundInsteadOfHold;
 
 	Tokenizer_TokenizeString(args, 0);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 2)) {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
 	iVal = Tokenizer_GetArgInteger(0);
 	bWrapAroundInsteadOfHold = Tokenizer_GetArgInteger(1);
@@ -1191,9 +1199,12 @@ static commandResult_t add_dimmer(const void *context, const char *cmd, const ch
 	int addMode;
 
 	Tokenizer_TokenizeString(args, 0);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
 	iVal = Tokenizer_GetArgInteger(0);
-	addMode = Tokenizer_GetArgInteger(1);
+	addMode = Tokenizer_GetArgIntegerDefault(1,0);
 
 	LED_AddDimmer(iVal, addMode, 0);
 
@@ -1204,6 +1215,9 @@ static commandResult_t dimmer(const void *context, const char *cmd, const char *
 		int iVal = 0;
 
         ADDLOG_DEBUG(LOG_FEATURE_CMD, " dimmer (%s) received with args %s",cmd,args);
+	if (!args || args[0] == '\0') {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
 		// according to Elektroda.com users, domoticz sends following string:
 		// {"brightness":52,"state":"ON"}
@@ -1383,6 +1397,9 @@ commandResult_t LED_SetBaseColor_HSB(const void *context, const char *cmd, const
 	const char *p;
 
 	Tokenizer_TokenizeString(args, 0);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {	// especially important here: if ArgsCount is 0, it would proceed to els, fetching 3 invalid values!!
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 	if (Tokenizer_GetArgsCount() == 1) {
 		p = args;
 		hue = atoi(p);
@@ -1583,6 +1600,9 @@ static commandResult_t lerpSpeed(const void *context, const char *cmd, const cha
 static commandResult_t cmdDimmerScale(const void *context, const char *cmd, const char *args, int cmdFlags) {
 	// Use tokenizer, so we can use variables (eg. $CH11 as variable)
 	Tokenizer_TokenizeString(args, 0);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
 	g_brightnessScale = Tokenizer_GetArgFloat(0);
 
@@ -1672,6 +1692,9 @@ static commandResult_t setSaturation(const void *context, const char *cmd, const
 
 	// Use tokenizer, so we can use variables (eg. $CH11 as variable)
 	Tokenizer_TokenizeString(args, 0);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
 	f = Tokenizer_GetArgFloat(0);
 
@@ -1692,6 +1715,9 @@ static commandResult_t setHue(const void *context, const char *cmd, const char *
 
 	// Use tokenizer, so we can use variables (eg. $CH11 as variable)
 	Tokenizer_TokenizeString(args, 0);
+	if (Tokenizer_CheckArgsCountAndPrintWarning(cmd, 1)) {
+		return CMD_RES_NOT_ENOUGH_ARGUMENTS;
+	}
 
 	f = Tokenizer_GetArgFloat(0);
 
diff --git a/src/cmnds/cmd_tokenizer.c b/src/cmnds/cmd_tokenizer.c
@@ -144,8 +144,9 @@ const char *Tokenizer_GetArgExpanding(int i) {
 const char *Tokenizer_GetArg(int i) {
 	const char *s;
 
-	if (i >= g_numArgs)
+	if (i < 0 || g_numArgs <= i) {
 		return 0;
+	}
 
 	if (g_argsExpanded[i][0] != 0) {
 		return g_argsExpanded[i];
@@ -200,6 +201,15 @@ const char *Tokenizer_GetArgFrom(int i) {
 }
 int Tokenizer_GetArgIntegerRange(int i, int rangeMin, int rangeMax) {
 	int ret = Tokenizer_GetArgInteger(i);
+
+//
+// to be discussed: What to return in case of an invalid index? min or max or ???
+//
+	if (i < 0 || g_numArgs <= i) {
+		ADDLOG_ERROR(LOG_FEATURE_CMD, "Invalid argument index %i! Using minumum value %i!",i,rangeMin);
+		return rangeMin;
+	}
+
 	if(ret < rangeMin) {
 		ret = rangeMin;
 		ADDLOG_ERROR(LOG_FEATURE_CMD, "Argument %i (val=%i) was out of range [%i,%i], clamped",i,ret,rangeMax,rangeMin);
@@ -227,7 +237,7 @@ int Tokenizer_GetPin(int i, int def) {
 int Tokenizer_GetArgIntegerDefault(int i, int def) {
 	int r;
 
-	if (g_numArgs <= i) {
+	if (i < 0 || g_numArgs <= i) {
 		return def;
 	}
 	r = Tokenizer_GetArgInteger(i);
@@ -237,7 +247,7 @@ int Tokenizer_GetArgIntegerDefault(int i, int def) {
 float Tokenizer_GetArgFloatDefault(int i, float def) {
 	float r;
 
-	if (g_numArgs <= i) {
+	if (i < 0 || g_numArgs <= i) {
 		return def;
 	}
 	r = Tokenizer_GetArgFloat(i);
@@ -247,6 +257,9 @@ float Tokenizer_GetArgFloatDefault(int i, float def) {
 int Tokenizer_GetArgInteger(int i) {
 	const char *s;
 	int ret;
+	if (i < 0 || g_numArgs <= i) {
+		return 0;
+	}
 
 	s = g_args[i];
 	if (s == 0)
@@ -278,6 +291,9 @@ int Tokenizer_GetArgInteger(int i) {
 	return atoi(s);
 }
 float Tokenizer_GetArgFloat(int i) {
+	if (i < 0 || g_numArgs <= i) {
+		return 0.0f;
+	}
 #if !ENABLE_EXPAND_CONSTANT
 	int channelIndex;
 #endif
diff --git a/src/selftest/selftest_local.h b/src/selftest/selftest_local.h
@@ -28,6 +28,7 @@ void SelfTest_Failed(const char *file, const char *function, int line, const cha
 #define SELFTEST_ASSERT_PIN_BOOLEAN(pinIndex, res) SELFTEST_ASSERT((SIM_GetSimulatedPinValue(pinIndex) == res));
 #define SELFTEST_ASSERT_ARGUMENT(argumentIndex, res) SELFTEST_ASSERT(!strcmp(Tokenizer_GetArg(argumentIndex), res));
 #define SELFTEST_ASSERT_ARGUMENT_INTEGER(argumentIndex, res) SELFTEST_ASSERT((Tokenizer_GetArgInteger(argumentIndex) == res));
+#define SELFTEST_ASSERT_ARGUMENT_FLOAT(argumentIndex, res) SELFTEST_ASSERT((Tokenizer_GetArgFloat(argumentIndex) == res));
 #define SELFTEST_ASSERT_ARGUMENTS_COUNT(wantedCount) SELFTEST_ASSERT((Tokenizer_GetArgsCount() == wantedCount));
 #define SELFTEST_ASSERT_JSON_VALUE_STRING(obj, varName, res) SELFTEST_ASSERT(!strcmp(Test_GetJSONValue_String(varName, obj), res));
 #define SELFTEST_ASSERT_JSON_ONE_OF_TWO_VALUES_STRING(obj, varName, res, res2) SELFTEST_ASSERT(!strcmp(Test_GetJSONValue_String(varName, obj), res) || !strcmp(Test_GetJSONValue_String(varName, obj), res2));
diff --git a/src/selftest/selftest_tokenizer.c b/src/selftest/selftest_tokenizer.c
@@ -135,6 +135,24 @@ void Test_Tokenizer() {
 	SELFTEST_ASSERT_ARGUMENT(2, "level=77");
 	SELFTEST_ASSERT_ARGUMENT(3, "0");
 	SELFTEST_ASSERT_STRING(Tokenizer_GetArgFrom(2), "level=$CH5 $CH6")
+	
+	// test for invalid responses
+	Tokenizer_TokenizeString("1 2 3 4 5 6.6 7.7", 0);
+	SELFTEST_ASSERT_ARGUMENTS_COUNT(7);
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(0, 1);
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(-1, 0);	// negative index: default 0
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(7, 0);		// invalid index: default 0
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(1024, 0);	// invalid index: default 0
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(1, 2);
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(2, 3);
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(3, 4);
+	SELFTEST_ASSERT_ARGUMENT_INTEGER(4, 5);
+	SELFTEST_ASSERT_ARGUMENT_FLOAT(5, 6.6f);
+	SELFTEST_ASSERT_ARGUMENT_FLOAT(6, 7.7f);
+	SELFTEST_ASSERT_ARGUMENT_FLOAT(-1, 0.0f);	// negative index: default 0.0
+	SELFTEST_ASSERT_ARGUMENT_FLOAT(7, 0.0f);	// invalid index: default 0.0
+	SELFTEST_ASSERT_ARGUMENT_FLOAT(1024, 0.0f);	// invalid index: default 0.0
+
 }
 
 #endif
