foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#27344: fuzz: Remove legacy int parse fuzz tests

Browse source 
faf8dc496e fuzz: Remove legacy int parse fuzz tests (MarcoFalke)

Pull request description:

  The fuzz tests checked that the result of the new function was equal to the legacy function. (Side note: The checks were incomplete, as evident by the follow-up fix in commit b5c9bb5cb9).

  Given that they haven't found any issues in years (beside missing the above issue, that they couldn't catch), it seems time to remove them.

  They may come in handy in the rare case that someone would want to modify `LocaleIndependentAtoi()` or `Parse*Int*()`, however that seems unlikely. Also, appropriate checks can be added then.

ACKs for top commit:
  fanquake:
    ACK faf8dc496e
  dergoegge:
    ACK faf8dc496e

Tree-SHA512: 4ec88b9fa8ba49a923b0604016f0f471b3c9b9e0ba6c5c3dc4e20503c6994789921e7221d9ec467a2a37a73f21a70ba51ba3370ed5ad311dee989e218290b29a
This commit is contained in:
fanquake 2023-03-28 11:49:36 +01:00
commit 8d31d769b7
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
3 changed files with 5 additions and 157 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
ci/test/06_script_b.sh
View file

@ -61,6 +61,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
" src/random.cpp"\ " src/random.cpp"\
" src/rpc/fees.cpp"\ " src/rpc/fees.cpp"\
" src/rpc/signmessage.cpp"\ " src/rpc/signmessage.cpp"\
" src/test/fuzz/string.cpp"\
" src/test/fuzz/txorphan.cpp"\ " src/test/fuzz/txorphan.cpp"\
" src/test/fuzz/util/"\ " src/test/fuzz/util/"\
" src/test/util/coins.cpp"\ " src/test/util/coins.cpp"\

159
src/test/fuzz/string.cpp
View file

@ -5,8 +5,6 @@
#include <blockfilter.h> #include <blockfilter.h>
#include <clientversion.h> #include <clientversion.h>
#include <common/url.h> #include <common/url.h>
#include <logging.h>
#include <netaddress.h>
#include <netbase.h> #include <netbase.h>
#include <outputtype.h> #include <outputtype.h>
#include <rpc/client.h> #include <rpc/client.h>
@ -22,113 +20,21 @@
#include <test/fuzz/util.h> #include <test/fuzz/util.h>
#include <util/error.h> #include <util/error.h>
#include <util/fees.h> #include <util/fees.h>
#include <util/message.h>
#include <util/settings.h> #include <util/settings.h>
#include <util/strencodings.h> #include <util/strencodings.h>
#include <util/string.h> #include <util/string.h>
#include <util/system.h> #include <util/system.h>
#include <util/translation.h> #include <util/translation.h>
#include <version.h>
#include <cassert>
#include <cstdint> #include <cstdint>
#include <cstdlib> #include <cstdlib>
#include <ios>
#include <stdexcept>
#include <string> #include <string>
#include <vector> #include <vector>
namespace { enum class FeeEstimateMode;
bool LegacyParsePrechecks(const std::string& str)
{
if (str.empty()) // No empty string allowed
return false;
if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size() - 1]))) // No padding allowed
return false;
if (!ContainsNoNUL(str)) // No embedded NUL characters allowed
return false;
return true;
}
bool LegacyParseInt32(const std::string& str, int32_t* out)
{
if (!LegacyParsePrechecks(str))
return false;
char* endp = nullptr;
errno = 0; // strtol will not set errno if valid
long int n = strtol(str.c_str(), &endp, 10);
if (out) *out = (int32_t)n;
// Note that strtol returns a *long int*, so even if strtol doesn't report an over/underflow
// we still have to check that the returned value is within the range of an *int32_t*. On 64-bit
// platforms the size of these types may be different.
return endp && *endp == 0 && !errno &&
n >= std::numeric_limits<int32_t>::min() &&
n <= std::numeric_limits<int32_t>::max();
}
bool LegacyParseInt64(const std::string& str, int64_t* out)
{
if (!LegacyParsePrechecks(str))
return false;
char* endp = nullptr;
errno = 0; // strtoll will not set errno if valid
long long int n = strtoll(str.c_str(), &endp, 10);
if (out) *out = (int64_t)n;
// Note that strtoll returns a *long long int*, so even if strtol doesn't report an over/underflow
// we still have to check that the returned value is within the range of an *int64_t*.
return endp && *endp == 0 && !errno &&
n >= std::numeric_limits<int64_t>::min() &&
n <= std::numeric_limits<int64_t>::max();
}
bool LegacyParseUInt32(const std::string& str, uint32_t* out)
{
if (!LegacyParsePrechecks(str))
return false;
if (str.size() >= 1 && str[0] == '-') // Reject negative values, unfortunately strtoul accepts these by default if they fit in the range
return false;
char* endp = nullptr;
errno = 0; // strtoul will not set errno if valid
unsigned long int n = strtoul(str.c_str(), &endp, 10);
if (out) *out = (uint32_t)n;
// Note that strtoul returns a *unsigned long int*, so even if it doesn't report an over/underflow
// we still have to check that the returned value is within the range of an *uint32_t*. On 64-bit
// platforms the size of these types may be different.
return endp && *endp == 0 && !errno &&
n <= std::numeric_limits<uint32_t>::max();
}
bool LegacyParseUInt8(const std::string& str, uint8_t* out)
{
uint32_t u32;
if (!LegacyParseUInt32(str, &u32) || u32 > std::numeric_limits<uint8_t>::max()) {
return false;
}
if (out != nullptr) {
*out = static_cast<uint8_t>(u32);
}
return true;
}
bool LegacyParseUInt64(const std::string& str, uint64_t* out)
{
if (!LegacyParsePrechecks(str))
return false;
if (str.size() >= 1 && str[0] == '-') // Reject negative values, unfortunately strtoull accepts these by default if they fit in the range
return false;
char* endp = nullptr;
errno = 0; // strtoull will not set errno if valid
unsigned long long int n = strtoull(str.c_str(), &endp, 10);
if (out) *out = (uint64_t)n;
// Note that strtoull returns a *unsigned long long int*, so even if it doesn't report an over/underflow
// we still have to check that the returned value is within the range of an *uint64_t*.
return endp && *endp == 0 && !errno &&
n <= std::numeric_limits<uint64_t>::max();
}
// For backwards compatibility checking.
int64_t atoi64_legacy(const std::string& str)
{
return strtoll(str.c_str(), nullptr, 10);
}
}; // namespace
FUZZ_TARGET(string) FUZZ_TARGET(string)
{ {
@ -236,61 +142,4 @@ FUZZ_TARGET(string)
const bilingual_str bs2{random_string_2, random_string_1}; const bilingual_str bs2{random_string_2, random_string_1};
(void)(bs1 + bs2); (void)(bs1 + bs2);
} }
{
int32_t i32;
int64_t i64;
uint32_t u32;
uint64_t u64;
uint8_t u8;
const bool ok_i32 = ParseInt32(random_string_1, &i32);
const bool ok_i64 = ParseInt64(random_string_1, &i64);
const bool ok_u32 = ParseUInt32(random_string_1, &u32);
const bool ok_u64 = ParseUInt64(random_string_1, &u64);
const bool ok_u8 = ParseUInt8(random_string_1, &u8);
int32_t i32_legacy;
int64_t i64_legacy;
uint32_t u32_legacy;
uint64_t u64_legacy;
uint8_t u8_legacy;
const bool ok_i32_legacy = LegacyParseInt32(random_string_1, &i32_legacy);
const bool ok_i64_legacy = LegacyParseInt64(random_string_1, &i64_legacy);
const bool ok_u32_legacy = LegacyParseUInt32(random_string_1, &u32_legacy);
const bool ok_u64_legacy = LegacyParseUInt64(random_string_1, &u64_legacy);
const bool ok_u8_legacy = LegacyParseUInt8(random_string_1, &u8_legacy);
assert(ok_i32 == ok_i32_legacy);
assert(ok_i64 == ok_i64_legacy);
assert(ok_u32 == ok_u32_legacy);
assert(ok_u64 == ok_u64_legacy);
assert(ok_u8 == ok_u8_legacy);
if (ok_i32) {
assert(i32 == i32_legacy);
}
if (ok_i64) {
assert(i64 == i64_legacy);
}
if (ok_u32) {
assert(u32 == u32_legacy);
}
if (ok_u64) {
assert(u64 == u64_legacy);
}
if (ok_u8) {
assert(u8 == u8_legacy);
}
}
{
const int locale_independent_atoi_result = LocaleIndependentAtoi<int>(random_string_1);
const int64_t atoi64_result = atoi64_legacy(random_string_1);
assert(locale_independent_atoi_result == std::clamp<int64_t>(atoi64_result, std::numeric_limits<int>::min(), std::numeric_limits<int>::max()));
}
{
const int64_t atoi64_result = atoi64_legacy(random_string_1);
const int64_t locale_independent_atoi_result = LocaleIndependentAtoi<int64_t>(random_string_1);
assert(atoi64_result == locale_independent_atoi_result);
}
} }

2
test/lint/lint-locale-dependence.py
View file

@ -44,8 +44,6 @@ from subprocess import check_output, CalledProcessError
KNOWN_VIOLATIONS = [ KNOWN_VIOLATIONS = [
"src/dbwrapper.cpp:.*vsnprintf", "src/dbwrapper.cpp:.*vsnprintf",
"src/test/fuzz/locale.cpp:.*setlocale", "src/test/fuzz/locale.cpp:.*setlocale",
"src/test/fuzz/string.cpp:.*strtol",
"src/test/fuzz/string.cpp:.*strtoul",
"src/test/util_tests.cpp:.*strtoll", "src/test/util_tests.cpp:.*strtoll",
"src/wallet/bdb.cpp:.*DbEnv::strerror", # False positive "src/wallet/bdb.cpp:.*DbEnv::strerror", # False positive
"src/util/syserror.cpp:.*strerror", # Outside this function use `SysErrorString` "src/util/syserror.cpp:.*strerror", # Outside this function use `SysErrorString`