Is there an existing issue for this?
- I have searched the existing issues
Current behaviour
When a function requires a const char *end, one might assume that the string doesn’t need to be null terminated. getJsonToken requires this parameter:
0enum jtokentype getJsonToken(std::string& tokenVal, unsigned int& consumed,
1 const char *raw, const char *end)
However, the function does assume that the underlying memory block is null-terminated as is visible here:
0 if (!strncmp(raw, "null", 4)) {
1 //...
2 } else if (!strncmp(raw, "true", 4)) {
3 //...
4 } else if (!strncmp(raw, "false", 5)) {
5 //...
and here:
0if ((*firstDigit == '0') && json_isdigit(firstDigit[1]))
and possibly others.
If the string isn’t null terminated, then we run risk of reading uninitialized memory, which could lead to a crash.
In practice this currently does not seem to lead to any issues, as all usages that I could find used underlying null-terminated blocks. It is however an uncomfortable pitfall for future development.
Expected behaviour
The function should either make sure to never access anything beyond or at the end pointer, or should document this requirement properly.
Steps to reproduce
0char innocentChar = 'n';
1getJsonToken(/*...*/, &innocentChar, (&innocentChar) + 1);
Relevant log output
No response
How did you obtain Bitcoin Core
Other
What version of Bitcoin Core are you using?
master@3654d84
Operating system and version
Common code issue, os irrelevant
Machine specifications
No response
