use byte-safe isalpha in epee json value parser
What the developer wrote
Authored by alhudz
use byte-safe isalpha in epee json value parser
What changed, and why it matters
This commit fixes a bug in Monero's JSON parser where it used the standard C 'isalpha' function to decide if a value looks like a keyword such as 'true', 'false', or 'null'. On some systems, passing a negative number (which can happen when the input contains high-byte non-ASCII characters like certain UTF-8 bytes) to the standard 'isalpha' can cause undefined behavior or be misclassified as alphabetic. The patch replaces the unsafe call with a custom byte-safe version that only looks at the raw byte value. A new test confirms that a value starting with a non-ASCII byte is now rejected instead of being treated as a keyword.
Apply the patch and run the new unit test. Review other uses of isalpha/isdigit in the epee parser and serialization code for similar byte-safety issues, especially where char values from untrusted network input are passed to C library character classification functions.
Security signals we found
Undefined behavior in standard isalpha() when passed negative char values
Potential parser misclassification of malformed UTF-8 input as JSON keyword literals
Locale-dependent character classification replaced with byte-safe lookup table
New unit test explicitly exercises rejection of non-ASCII leading byte in value position
Evidence from the diff
The epee JSON value parser previously relied on the locale-dependent C library isalpha() to detect keyword literals (true/false/null). Because char may be signed, high-byte UTF-8 continuation bytes (e.g., 0xC3 0x28) can yield negative values; passing negative values to isalpha() is undefined behavior per the C standard and on common glibc implementations can be misclassified as alphabetic, causing the parser to accept malformed input as a boolean/null keyword. The patch adds a custom isalpha() helper in parserse_base_utils.h that indexes a lookup table with (uint8_t)c, making it byte-safe and locale-independent, and updates portable_storage_from_json.h to use it. A unit test verifies that a JSON value beginning with a non-ASCII byte is rejected.
Changed components
contrib/epee/include/storages/parserse_base_utils.hcontrib/epee/include/storages/portable_storage_from_json.htests/unit_tests/epee_serialization.cppInspect captured patch +37 / −3
diff --git a/contrib/epee/include/storages/parserse_base_utils.h b/contrib/epee/include/storages/parserse_base_utils.h
index 4658eba..2f27e08 100644
--- a/contrib/epee/include/storages/parserse_base_utils.h
+++ b/contrib/epee/include/storages/parserse_base_utils.h
@@ -93,6 +93,11 @@ namespace misc_utils
return lut[(uint8_t)c] & 1;
}
+ inline bool isalpha(char c)
+ {
+ return lut[(uint8_t)c] & 4;
+ }
+
std::string transform_to_escape_sequence(const std::string& src);
/*
diff --git a/contrib/epee/include/storages/portable_storage_from_json.h b/contrib/epee/include/storages/portable_storage_from_json.h
index 60acfcc..8acb5b4 100644
--- a/contrib/epee/include/storages/portable_storage_from_json.h
+++ b/contrib/epee/include/storages/portable_storage_from_json.h
@@ -146,7 +146,7 @@ namespace epee
stg.set_value(name, double(nval), current_section);
}
state = match_state_wonder_after_value;
- }else if(isalpha(*it) )
+ }else if(epee::misc_utils::parse::isalpha(*it) )
{// could be null, true or false
boost::string_ref word;
misc_utils::parse::match_word2(it, buf_end, word);
@@ -245,7 +245,7 @@ namespace epee
{
array_md = array_mode_undifined;
state = match_state_wonder_after_value;
- }else if(isalpha(*it) )
+ }else if(epee::misc_utils::parse::isalpha(*it) )
{// array of booleans
boost::string_ref word;
misc_utils::parse::match_word2(it, buf_end, word);
@@ -333,7 +333,7 @@ namespace epee
}else CHECK_ISSPACE();
break;
case array_mode_booleans:
- if(isalpha(*it) )
+ if(epee::misc_utils::parse::isalpha(*it) )
{// array of booleans
boost::string_ref word;
misc_utils::parse::match_word2(it, buf_end, word);
diff --git a/tests/unit_tests/epee_serialization.cpp b/tests/unit_tests/epee_serialization.cpp
index 2cafc0e..ec39996 100644
--- a/tests/unit_tests/epee_serialization.cpp
+++ b/tests/unit_tests/epee_serialization.cpp
@@ -98,6 +98,35 @@ struct ObjWithOptChild
KV_SERIALIZE_OPT(test_value, true);
END_KV_SERIALIZE_MAP()
};
+
+struct ObjWithBool
+{
+ bool b;
+
+ BEGIN_KV_SERIALIZE_MAP()
+ KV_SERIALIZE(b)
+ END_KV_SERIALIZE_MAP()
+};
+}
+
+TEST(epee_json, keyword_values)
+{
+ // true/false/null parse as keyword values
+ ObjWithBool o{};
+
+ o.b = false;
+ EXPECT_TRUE(epee::serialization::load_t_from_json(o, std::string("{\"b\": true}")));
+ EXPECT_TRUE(o.b);
+
+ o.b = true;
+ EXPECT_TRUE(epee::serialization::load_t_from_json(o, std::string("{\"b\": false}")));
+ EXPECT_FALSE(o.b);
+
+ EXPECT_TRUE(epee::serialization::load_t_from_json(o, std::string("{\"b\": null}")));
+
+ // A value beginning with a non-ASCII byte (most significant bit == 1) is not
+ // a keyword and must be rejected.
+ EXPECT_FALSE(epee::serialization::load_t_from_json(o, std::string("{\"b\": \xc3\x28}")));
}
TEST(epee_binary, serialize_deserialize)
Why this scored 51/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.