Skip to content

Commit 28fee85

Browse files
zmurzmur
committed
Bug#33589961 Invalid numeric values such as '10 "M12L"' should not be allowed in config.ini
Cleanup up of InitConfigFileParser::convertStringToUint64 function. Strange numeric suffix including extra characters or quotes or spaces between number and suffix is no longer supported on numeric values in config.ini. For example OverloadLimit=10 "M12L" OverloadLimit=10 M was allowed and treated as OverloadLimit=10M Also bare suffix was allowed on some platforms and treated as zero OverloadLimit=MAX_UINT that will now be rejected on all platforms. Note, this fix may now cause rejection of config.ini that was interpreted correctly although using unsupported syntax. This is expected to very very rare. These cases should be detected when running ndb_mgmd reading from config.ini and it should be simple to correct the configuration file. Change-Id: Ic6235cbcca9fddb6d5fffb55d0ca695a819d1c12
1 parent 3956a81 commit 28fee85

File tree

3 files changed

+126
-31
lines changed

3 files changed

+126
-31
lines changed

‎storage/ndb/src/mgmsrv/CMakeLists.txt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,7 @@ IF(UNIX)
7171
TARGET_LINK_LIBRARIES(ndb_mgmd ${EDITLINE_LIBRARY})
7272
ENDIF(UNIX)
7373

74-
NDB_ADD_TEST(MgmConfig-t testConfig.cpp LIBS ndbmgmapi ndbtrace ndbconf ndblogger ndbgeneral ndbportlib)
74+
NDB_ADD_TEST(MgmConfig-t testConfig.cpp
75+
LIBS ndbmgmapi ndbtrace ndbconf ndblogger ndbgeneral ndbportlib)
76+
NDB_ADD_TEST(InitConfigFileParser-t InitConfigFileParser.cpp
77+
LIBS ndbconf ndbgeneral ndbportlib)

‎storage/ndb/src/mgmsrv/InitConfigFileParser.cpp‎

Lines changed: 121 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <m_string.h>
3434
#include <util/SparseBitmask.hpp>
3535
#include "../common/util/parse_mask.hpp"
36+
#include <stdlib.h>
3637

3738
extern EventLogger *g_eventLogger;
3839

@@ -425,42 +426,78 @@ bool InitConfigFileParser::isEmptyLine(const char* line) const {
425426
// Convert String to Int
426427
//****************************************************************************
427428
bool InitConfigFileParser::convertStringToUint64(const char* s,
428-
Uint64& val,
429-
Uint32 log10base) {
429+
Uint64& val)
430+
{
430431
if (s == NULL)
431432
return false;
432-
if (strlen(s) == 0)
433-
return false;
434433

435-
errno = 0;
434+
while (*s != '\0' && isspace(*s)) s++;
435+
const bool negative = (*s == '-');
436+
437+
union {
438+
signed long long vs;
439+
unsigned long long vu;
440+
};
436441
char* p;
437-
Int64 v = my_strtoll(s, &p, log10base);
438-
if (errno != 0)
439-
return false;
440-
441-
long mul = 0;
442-
if (p != &s[strlen(s)]){
443-
char * tmp = strdup(p);
444-
trim(tmp);
445-
switch(tmp[0]){
446-
case 'k':
447-
case 'K':
448-
mul = 10;
449-
break;
450-
case 'M':
451-
mul = 20;
452-
break;
453-
case 'G':
454-
mul = 30;
455-
break;
456-
default:
457-
free(tmp);
442+
const int log10base = 0;
443+
errno = 0;
444+
if (negative)
445+
{
446+
vs = strtoll(s, &p, log10base);
447+
if ((vs == LLONG_MIN || vs == LLONG_MAX) && errno == ERANGE)
448+
{
458449
return false;
459450
}
460-
free(tmp);
461451
}
462-
463-
val = (v << mul);
452+
else
453+
{
454+
vu = strtoull(s, &p, log10base);
455+
if (vu == ULLONG_MAX && errno == ERANGE)
456+
{
457+
return false;
458+
}
459+
}
460+
if (p == s)
461+
return false;
462+
463+
int mul = 0;
464+
465+
switch(*p)
466+
{
467+
case '\0':
468+
break;
469+
case 'k':
470+
case 'K':
471+
mul = 10;
472+
p++;
473+
break;
474+
case 'M':
475+
mul = 20;
476+
p++;
477+
break;
478+
case 'G':
479+
mul = 30;
480+
p++;
481+
break;
482+
default:
483+
return false;
484+
}
485+
if (*p != '\0')
486+
return false;
487+
if (negative)
488+
{
489+
Int64 v = (vs << mul);
490+
if ((v >> mul) != vs)
491+
return false;
492+
val = v;
493+
}
494+
else
495+
{
496+
Uint64 v = (vu << mul);
497+
if ((v >> mul) != vu)
498+
return false;
499+
val = v;
500+
}
464501
return true;
465502
}
466503

@@ -1037,3 +1074,58 @@ template class Vector<struct my_option>;
10371074
/*
10381075
See include/my_getopt.h for the declaration of struct my_option
10391076
*/
1077+
1078+
1079+
#ifdef TEST_INITCONFIGFILEPARSER
1080+
1081+
#include "../../../../unittest/mytap/tap.h"
1082+
1083+
int main()
1084+
{
1085+
plan(29);
1086+
1087+
// Check the string to int parsing function convertStringToUint64()
1088+
Uint64 value;
1089+
ok1(InitConfigFileParser::convertStringToUint64("37", value));
1090+
ok1(value == 37);
1091+
ok1(InitConfigFileParser::convertStringToUint64("0", value));
1092+
ok1(value == 0);
1093+
ok1(InitConfigFileParser::convertStringToUint64("-0", value)); // ??
1094+
ok1(static_cast<Int64>(value) == -0);
1095+
ok1(InitConfigFileParser::convertStringToUint64("-1", value));
1096+
ok1(static_cast<Int64>(value) == -1);
1097+
ok1(InitConfigFileParser::convertStringToUint64("-37", value));
1098+
ok1(static_cast<Int64>(value) == -37);
1099+
1100+
// Valid multipliers
1101+
ok1(InitConfigFileParser::convertStringToUint64("37k", value));
1102+
ok1(value == 37ull * 1024);
1103+
ok1(InitConfigFileParser::convertStringToUint64("37K", value));
1104+
ok1(value == 37ull * 1024);
1105+
ok1(InitConfigFileParser::convertStringToUint64("37M", value));
1106+
ok1(value == 37ull * 1024 * 1024);
1107+
ok1(InitConfigFileParser::convertStringToUint64("37G", value));
1108+
ok1(value == 37ull * 1024 * 1024 * 1024);
1109+
#ifdef NOT_YET
1110+
ok1(InitConfigFileParser::convertStringToUint64("37T", value));
1111+
ok1(value == 37ull * 1024 * 1024 * 1024 * 1024);
1112+
#endif
1113+
1114+
// Invalid multipliers
1115+
ok1(InitConfigFileParser::convertStringToUint64("10kB", value) == false);
1116+
ok1(InitConfigFileParser::convertStringToUint64("10 M", value) == false);
1117+
ok1(InitConfigFileParser::convertStringToUint64("10 \"M12L\"", value) == false);
1118+
ok1(InitConfigFileParser::convertStringToUint64("10 'M12L'", value) == false);
1119+
ok1(InitConfigFileParser::convertStringToUint64("10M12L", value) == false);
1120+
ok1(InitConfigFileParser::convertStringToUint64("10T'", value) == false);
1121+
1122+
ok1(InitConfigFileParser::convertStringToUint64("M", value) == false);
1123+
ok1(InitConfigFileParser::convertStringToUint64("MAX_UINT", value) == false);
1124+
ok1(InitConfigFileParser::convertStringToUint64("Magnus'", value) == false);
1125+
ok1(InitConfigFileParser::convertStringToUint64("", value) == false);
1126+
ok1(InitConfigFileParser::convertStringToUint64("trettisju", value) == false);
1127+
1128+
return exit_status();
1129+
}
1130+
1131+
#endif

‎storage/ndb/src/mgmsrv/InitConfigFileParser.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class InitConfigFileParser {
9898
ATTRIBUTE_FORMAT(printf, 2, 3);
9999
};
100100

101-
static bool convertStringToUint64(const char* s, Uint64& val, Uint32 log10base = 0);
101+
static bool convertStringToUint64(const char* s, Uint64& val);
102102
static bool convertStringToBool(const char* s, bool& val);
103103

104104
private:

0 commit comments

Comments
 (0)