mirror of
https://github.com/meshcore-dev/MeshCore.git
synced 2026-10-28 16:13:32 +00:00
fix(radio): refuse "nan" for the airtime factor
"set af nan" is accepted and stores a NaN in airtime_factor, the duty cycle budget the roles hand to their airtime accounting. strtof() consumes the token, so the parse guard does not fire, and no ordered comparison is true for a NaN, so "af < 0 || af > 9" cannot reject it. "set dutycycle" writes the same field and has the same hole. Both commands now check the text before it is parsed: after optional white space and sign, a number has to start with a digit or a '.'. That refuses "nan", "inf" and their variants, and keeps every spelling that was accepted before (signs, leading white space, ".5"). The ranges and both reply strings are unchanged. The check is on the text rather than on the parsed float on purpose. nRF52 builds use -Ofast, and under -ffast-math the compiler may assume a float is never NaN or infinite. A test on the value, even one that reads its bits, can then be folded away: clang does exactly that with a bit test at -Ofast, while arm-none-eabi-gcc keeps it. A character test does not depend on that.
This commit is contained in:
1 file changed
+18
-2
@@ -2,6 +2,22 @@
|
||||
#include "TxtDataHelpers.h"
|
||||
#include "Utils.h"
|
||||
#include "target.h"
|
||||
#include <ctype.h>
|
||||
|
||||
namespace {
|
||||
|
||||
// strtof() and atof() also read "nan" and "inf". A NaN cannot be refused by a
|
||||
// range check: no ordered comparison is true for it, and under -ffast-math,
|
||||
// which nRF52 builds use, the compiler may assume it never occurs and fold any
|
||||
// test on the parsed value away. So the text is checked before it is parsed:
|
||||
// after optional white space and sign, a number starts with a digit or a '.'.
|
||||
bool looksLikeANumber(const char* s) {
|
||||
while (isspace((unsigned char)*s)) s++;
|
||||
if (*s == '+' || *s == '-') s++;
|
||||
return isdigit((unsigned char)*s) || *s == '.';
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
bool CommonRadioPrefs::getByKey(const char* key, char* value, size_t max_len) {
|
||||
if (strcmp(key, "fem_rxgain") == 0) {
|
||||
@@ -69,7 +85,7 @@ bool CommonRadioPrefs::handleCommand(const char* command, uint32_t sender_timest
|
||||
if (memcmp(command, "set af ", 7) == 0) {
|
||||
char* end;
|
||||
float af = strtof(&command[7], &end);
|
||||
if (end == &command[7] || af < 0 || af > 9) {
|
||||
if (end == &command[7] || !looksLikeANumber(&command[7]) || af < 0 || af > 9) {
|
||||
strcpy(reply, "ERROR: af must be 0-9");
|
||||
} else {
|
||||
setAirtimeFactor(af);
|
||||
@@ -87,7 +103,7 @@ bool CommonRadioPrefs::handleCommand(const char* command, uint32_t sender_timest
|
||||
}
|
||||
if (memcmp(command, "set dutycycle ", 14) == 0) {
|
||||
float dc = atof(&command[14]);
|
||||
if (dc < 1 || dc > 100) {
|
||||
if (!looksLikeANumber(&command[14]) || dc < 1 || dc > 100) {
|
||||
strcpy(reply, "ERROR: dutycycle must be 1-100");
|
||||
} else {
|
||||
setAirtimeFactor((100.0f / dc) - 1.0f);
|
||||
|
||||
Reference in new issue
Block a user