Skip negative integers in input framework where not type-permissible

Script-layer counts, when provided as negative integers in an input
file, got cast to unsigned values because strtoull() does not complain
about negative values. For example, input string "-1" would lead to
value 18446744073709551615 (an all-ones 64-bit int) on x86_64. This is
more likely to be an error than an intent to get very large,
platform-dependent values, so these input lines are now skipped with
according messaging in the reporter.log/stderr.

This also affected ports: -1/tcp got cast to unsigned and only thrown
out because PortVal rejects values > 65535, mapping them to 0. We now
skip such inputs as well.

Updates existing input framework tests to capture the new behavior.
This commit is contained in:
Christian Kreibich 2021-01-28 22:42:39 -08:00
parent d845f01b27
commit 38d6b6a98b
7 changed files with 25 additions and 7 deletions

View file

@ -261,7 +261,7 @@ Value* Ascii::ParseValue(const string& s, const string& name, TypeTag type, Type
case TYPE_COUNT: case TYPE_COUNT:
val->val.uint_val = strtoull(start, &end, 10); val->val.uint_val = strtoull(start, &end, 10);
if ( CheckNumberError(start, end) ) if ( CheckNumberError(start, end, true) )
goto parse_error; goto parse_error;
break; break;
@ -292,7 +292,7 @@ Value* Ascii::ParseValue(const string& s, const string& name, TypeTag type, Type
start = numberpart.c_str(); start = numberpart.c_str();
} }
val->val.port_val.port = strtoull(start, &end, 10); val->val.port_val.port = strtoull(start, &end, 10);
if ( CheckNumberError(start, end) ) if ( CheckNumberError(start, end, true) )
goto parse_error; goto parse_error;
} }
break; break;
@ -473,7 +473,7 @@ parse_error:
return nullptr; return nullptr;
} }
bool Ascii::CheckNumberError(const char* start, const char* end) const bool Ascii::CheckNumberError(const char* start, const char* end, bool nonneg_only) const
{ {
MsgThread* thread = GetThread(); MsgThread* thread = GetThread();
@ -491,6 +491,18 @@ bool Ascii::CheckNumberError(const char* start, const char* end) const
if ( (*end != '\0') ) if ( (*end != '\0') )
thread->Warning(thread->Fmt("Number '%s' contained non-numeric trailing characters. Ignored trailing characters '%s'", start, end)); thread->Warning(thread->Fmt("Number '%s' contained non-numeric trailing characters. Ignored trailing characters '%s'", start, end));
if ( nonneg_only ) {
// String may legitimately start with whitespace, so
// we skip this before checking for a minus sign.
const char* s = start;
while ( s < end && isspace(*s) )
s++;
if ( *s == '-' ) {
thread->Warning(thread->Fmt("Number '%s' cannot be negative", start));
return true;
}
}
if ( errno == EINVAL ) if ( errno == EINVAL )
{ {
thread->Warning(thread->Fmt("String '%s' could not be converted to a number", start)); thread->Warning(thread->Fmt("String '%s' could not be converted to a number", start));

View file

@ -54,7 +54,7 @@ public:
TypeTag type, TypeTag subtype = TYPE_ERROR) const; TypeTag type, TypeTag subtype = TYPE_ERROR) const;
private: private:
bool CheckNumberError(const char* start, const char* end) const; bool CheckNumberError(const char* start, const char* end, bool nonneg_only = false) const;
SeparatorInfo separators; SeparatorInfo separators;
}; };

View file

@ -5,5 +5,7 @@ warning: ..<...>/Input::READER_ASCII: Number '9223372036854775801TEXTHERE' conta
warning: ..<...>/Input::READER_ASCII: Number '1Justtext' contained non-numeric trailing characters. Ignored trailing characters 'Justtext' warning: ..<...>/Input::READER_ASCII: Number '1Justtext' contained non-numeric trailing characters. Ignored trailing characters 'Justtext'
warning: ..<...>/Input::READER_ASCII: String 'Justtext' contained no parseable number warning: ..<...>/Input::READER_ASCII: String 'Justtext' contained no parseable number
warning: ..<...>/Input::READER_ASCII: Could not convert line 'Justtext 1' of ../input.log to Val. Ignoring line. warning: ..<...>/Input::READER_ASCII: Could not convert line 'Justtext 1' of ../input.log to Val. Ignoring line.
warning: ..<...>/Input::READER_ASCII: Number ' -18446744073709551612' cannot be negative
warning: ..<...>/Input::READER_ASCII: Could not convert line '9223372036854775800 -18446744073709551612' of ../input.log to Val. Ignoring line.
received termination signal received termination signal
>>> >>>

View file

@ -1,5 +1,4 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
{ {
[9223372036854775801] = [c=1], [9223372036854775801] = [c=1]
[9223372036854775800] = [c=4]
} }

View file

@ -1,3 +1,5 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
warning: ../input.log/Input::READER_ASCII: Port '50/trash' contained unknown protocol 'trash' warning: ../input.log/Input::READER_ASCII: Port '50/trash' contained unknown protocol 'trash'
warning: ../input.log/Input::READER_ASCII: Number '-1' cannot be negative
warning: ../input.log/Input::READER_ASCII: Could not convert line '1.2.3.8 -1/tcp' of ../input.log to Val. Ignoring line.
received termination signal received termination signal

View file

@ -4,6 +4,8 @@
# @TEST-EXEC: sed 1d .stderr > .stderrwithoutfirstline # @TEST-EXEC: sed 1d .stderr > .stderrwithoutfirstline
# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderrwithoutfirstline # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderrwithoutfirstline
# Note the tab+space separator in the last line of the following is
# intentional. It verifies our whitespace handling.
@TEST-START-FILE input.log @TEST-START-FILE input.log
#separator \x09 #separator \x09
#fields i c #fields i c

View file

@ -9,6 +9,7 @@
1.2.3.5 52/udp 1.2.3.5 52/udp
1.2.3.6 30/unknown 1.2.3.6 30/unknown
1.2.3.7 50/trash 1.2.3.7 50/trash
1.2.3.8 -1/tcp
@TEST-END-FILE @TEST-END-FILE
redef exit_only_after_terminate = T; redef exit_only_after_terminate = T;