Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31391: util: Drop boost posix_time in ParseISO8…
Browse files Browse the repository at this point in the history
…601DateTime

faf70cc Remove wallet::ParseISO8601DateTime, use ParseISO8601DateTime instead (MarcoFalke)
2222aec util: Implement ParseISO8601DateTime based on C++20 (MarcoFalke)

Pull request description:

  `boost::posix_time` in `ParseISO8601DateTime` has many issues:

  * It parses random strings that are clearly invalid and returns a time value for them, see [1] below.
  * None of the separators `-`, or `:`, or `T`, or `Z` are validated.
  * It may crash when running under a hardened C++ library, see bitcoin/bitcoin#28917.
  * It has been unmaintained for years, so reporting or fixing any issues will most likely be useless.
  * It pulls in a third-party dependency, when the functionality is already included in vanilla C++20.

  Fix all issues by replacing it with a simple helper function written in C++20.

  Fixes bitcoin/bitcoin#28917.

  [1] The following patch passes on current master:

  ```diff
  diff --git a/src/wallet/test/rpc_util_tests.cpp b/src/wallet/test/rpc_util_tests.cpp
  index 32f6f5ab46..c1c94c7116 100644
  --- a/src/wallet/test/rpc_util_tests.cpp
  +++ b/src/wallet/test/rpc_util_tests.cpp
  @@ -12,6 +12,14 @@ BOOST_AUTO_TEST_SUITE(wallet_util_tests)

   BOOST_AUTO_TEST_CASE(util_ParseISO8601DateTime)
   {
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("964296"), 242118028800);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("244622"), 15023836800);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("+INfINITy"), 9223372036854);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("7000802 01"), 158734166400);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("7469-2 +INfINITy"), 9223372036854);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("maXimum-datE-time"), 253402300799);
  +    BOOST_CHECK_EQUAL(ParseISO8601DateTime("577737     114maXimum-datE-time"), 253402300799);
  +
       BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z"), 0);
       BOOST_CHECK_EQUAL(ParseISO8601DateTime("1960-01-01T00:00:00Z"), 0);
       BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:00:01Z"), 946684801);
  ```

ACKs for top commit:
  hebasto:
    ACK faf70cc, I have reviewed the code and it looks OK.
  dergoegge:
    utACK faf70cc

Tree-SHA512: 9dd745a356d04acf6200e13a6af52c51a9e2a0eeccea110093ce5da147b3c669c0eda918e46db0164c081a78c8feae3fe557a4759bea18449a8ff2d090095931
  • Loading branch information
fanquake committed Dec 4, 2024
2 parents ff873a2 + faf70cc commit ae69fc3
Show file tree
Hide file tree
Showing 14 changed files with 94 additions and 69 deletions.
1 change: 1 addition & 0 deletions src/test/fuzz/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ add_executable(fuzz
p2p_transport_serialization.cpp
package_eval.cpp
parse_hd_keypath.cpp
parse_iso8601.cpp
parse_numbers.cpp
parse_script.cpp
parse_univalue.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// Copyright (c) 2019-2022 The Bitcoin Core developers
// Copyright (c) 2019-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <util/time.h>
#include <wallet/rpc/util.h>

#include <cassert>
#include <cstdint>
Expand All @@ -21,14 +20,8 @@ FUZZ_TARGET(parse_iso8601)

const std::string iso8601_datetime = FormatISO8601DateTime(random_time);
(void)FormatISO8601Date(random_time);
const int64_t parsed_time_1 = wallet::ParseISO8601DateTime(iso8601_datetime);
if (random_time >= 0) {
assert(parsed_time_1 >= 0);
if (iso8601_datetime.length() == 20) {
assert(parsed_time_1 == random_time);
}
}
const int64_t parsed_time_1{ParseISO8601DateTime(iso8601_datetime).value()};
assert(parsed_time_1 == random_time);

const int64_t parsed_time_2 = wallet::ParseISO8601DateTime(random_string);
assert(parsed_time_2 >= 0);
(void)ParseISO8601DateTime(random_string);
}
6 changes: 3 additions & 3 deletions src/test/fuzz/util.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2021-2022 The Bitcoin Core developers
// Copyright (c) 2021-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand Down Expand Up @@ -34,8 +34,8 @@ CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::option
int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional<int64_t>& min, const std::optional<int64_t>& max) noexcept
{
// Avoid t=0 (1970-01-01T00:00:00Z) since SetMockTime(0) disables mocktime.
static const int64_t time_min{946684801}; // 2000-01-01T00:00:01Z
static const int64_t time_max{4133980799}; // 2100-12-31T23:59:59Z
static const int64_t time_min{ParseISO8601DateTime("2000-01-01T00:00:01Z").value()};
static const int64_t time_max{ParseISO8601DateTime("2100-12-31T23:59:59Z").value()};
return fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(min.value_or(time_min), max.value_or(time_max));
}

Expand Down
51 changes: 48 additions & 3 deletions src/test/util_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011-2022 The Bitcoin Core developers
// Copyright (c) 2011-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand Down Expand Up @@ -323,20 +323,65 @@ BOOST_AUTO_TEST_CASE(util_TrimString)
BOOST_CHECK_EQUAL(TrimStringView(std::string("\x05\x04\x03\x02\x01\x00", 6), std::string("\x05\x04\x03\x02\x01\x00", 6)), "");
}

BOOST_AUTO_TEST_CASE(util_ParseISO8601DateTime)
{
BOOST_CHECK_EQUAL(ParseISO8601DateTime("1969-12-31T23:59:59Z").value(), -1);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z").value(), 0);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:01Z").value(), 1);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:00:01Z").value(), 946684801);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2011-09-30T23:36:17Z").value(), 1317425777);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2100-12-31T23:59:59Z").value(), 4133980799);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("9999-12-31T23:59:59Z").value(), 253402300799);

// Accept edge-cases, where the time overflows. They are not produced by
// FormatISO8601DateTime, so this can be changed in the future, if needed.
// For now, keep compatibility with the previous implementation.
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T99:00:00Z").value(), 947041200);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:99:00Z").value(), 946690740);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:00:99Z").value(), 946684899);
BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T99:99:99Z").value(), 947047239);

// Reject date overflows.
BOOST_CHECK(!ParseISO8601DateTime("2000-99-01T00:00:00Z"));
BOOST_CHECK(!ParseISO8601DateTime("2000-01-99T00:00:00Z"));

// Reject out-of-range years
BOOST_CHECK(!ParseISO8601DateTime("32768-12-31T23:59:59Z"));
BOOST_CHECK(!ParseISO8601DateTime("32767-12-31T23:59:59Z"));
BOOST_CHECK(!ParseISO8601DateTime("32767-12-31T00:00:00Z"));
BOOST_CHECK(!ParseISO8601DateTime("999-12-31T00:00:00Z"));

// Reject invalid format
const std::string valid{"2000-01-01T00:00:01Z"};
BOOST_CHECK(ParseISO8601DateTime(valid).has_value());
for (auto mut{0U}; mut < valid.size(); ++mut) {
std::string invalid{valid};
invalid[mut] = 'a';
BOOST_CHECK(!ParseISO8601DateTime(invalid));
}
}

BOOST_AUTO_TEST_CASE(util_FormatISO8601DateTime)
{
BOOST_CHECK_EQUAL(FormatISO8601DateTime(971890963199), "32767-12-31T23:59:59Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(971890876800), "32767-12-31T00:00:00Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(1317425777), "2011-09-30T23:36:17Z");

BOOST_CHECK_EQUAL(FormatISO8601DateTime(-1), "1969-12-31T23:59:59Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(0), "1970-01-01T00:00:00Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(1), "1970-01-01T00:00:01Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(946684801), "2000-01-01T00:00:01Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(1317425777), "2011-09-30T23:36:17Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(4133980799), "2100-12-31T23:59:59Z");
BOOST_CHECK_EQUAL(FormatISO8601DateTime(253402300799), "9999-12-31T23:59:59Z");
}

BOOST_AUTO_TEST_CASE(util_FormatISO8601Date)
{
BOOST_CHECK_EQUAL(FormatISO8601Date(971890963199), "32767-12-31");
BOOST_CHECK_EQUAL(FormatISO8601Date(971890876800), "32767-12-31");
BOOST_CHECK_EQUAL(FormatISO8601Date(1317425777), "2011-09-30");

BOOST_CHECK_EQUAL(FormatISO8601Date(0), "1970-01-01");
BOOST_CHECK_EQUAL(FormatISO8601Date(1317425777), "2011-09-30");
}

BOOST_AUTO_TEST_CASE(util_FormatMoney)
Expand Down
29 changes: 28 additions & 1 deletion src/util/time.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand All @@ -8,10 +8,13 @@
#include <compat/compat.h>
#include <tinyformat.h>
#include <util/check.h>
#include <util/strencodings.h>

#include <atomic>
#include <chrono>
#include <optional>
#include <string>
#include <string_view>
#include <thread>

void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); }
Expand Down Expand Up @@ -60,6 +63,30 @@ std::string FormatISO8601Date(int64_t nTime)
return strprintf("%04i-%02u-%02u", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()});
}

std::optional<int64_t> ParseISO8601DateTime(std::string_view str)
{
constexpr auto FMT_SIZE{std::string_view{"2000-01-01T01:01:01Z"}.size()};
if (str.size() != FMT_SIZE || str[4] != '-' || str[7] != '-' || str[10] != 'T' || str[13] != ':' || str[16] != ':' || str[19] != 'Z') {
return {};
}
const auto year{ToIntegral<uint16_t>(str.substr(0, 4))};
const auto month{ToIntegral<uint8_t>(str.substr(5, 2))};
const auto day{ToIntegral<uint8_t>(str.substr(8, 2))};
const auto hour{ToIntegral<uint8_t>(str.substr(11, 2))};
const auto min{ToIntegral<uint8_t>(str.substr(14, 2))};
const auto sec{ToIntegral<uint8_t>(str.substr(17, 2))};
if (!year || !month || !day || !hour || !min || !sec) {
return {};
}
const std::chrono::year_month_day ymd{std::chrono::year{*year}, std::chrono::month{*month}, std::chrono::day{*day}};
if (!ymd.ok()) {
return {};
}
const auto time{std::chrono::hours{*hour} + std::chrono::minutes{*min} + std::chrono::seconds{*sec}};
const auto tp{std::chrono::sys_days{ymd} + time};
return int64_t{TicksSinceEpoch<std::chrono::seconds>(tp)};
}

struct timeval MillisToTimeval(int64_t nTimeout)
{
struct timeval timeout;
Expand Down
5 changes: 4 additions & 1 deletion src/util/time.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand All @@ -8,7 +8,9 @@

#include <chrono> // IWYU pragma: export
#include <cstdint>
#include <optional>
#include <string>
#include <string_view>

using namespace std::chrono_literals;

Expand Down Expand Up @@ -105,6 +107,7 @@ T GetTime()
*/
std::string FormatISO8601DateTime(int64_t nTime);
std::string FormatISO8601Date(int64_t nTime);
std::optional<int64_t> ParseISO8601DateTime(std::string_view str);

/**
* Convert milliseconds to a struct timeval for e.g. select.
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/rpc/backup.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand Down Expand Up @@ -549,7 +549,7 @@ RPCHelpMan importwallet()
continue;
CKey key = DecodeSecret(vstr[0]);
if (key.IsValid()) {
int64_t nTime = ParseISO8601DateTime(vstr[1]);
int64_t nTime{ParseISO8601DateTime(vstr[1]).value_or(0)};
std::string strLabel;
bool fLabel = true;
for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) {
Expand All @@ -569,7 +569,7 @@ RPCHelpMan importwallet()
} else if(IsHex(vstr[0])) {
std::vector<unsigned char> vData(ParseHex(vstr[0]));
CScript script = CScript(vData.begin(), vData.end());
int64_t birth_time = ParseISO8601DateTime(vstr[1]);
int64_t birth_time{ParseISO8601DateTime(vstr[1]).value_or(0)};
if (birth_time > 0) nTimeBegin = std::min(nTimeBegin, birth_time);
scripts.emplace_back(script, birth_time);
}
Expand Down
18 changes: 1 addition & 17 deletions src/wallet/rpc/util.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011-2022 The Bitcoin Core developers
// Copyright (c) 2011-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand All @@ -14,26 +14,10 @@
#include <string_view>
#include <univalue.h>

#include <boost/date_time/posix_time/posix_time.hpp>

namespace wallet {
static const std::string WALLET_ENDPOINT_BASE = "/wallet/";
const std::string HELP_REQUIRING_PASSPHRASE{"\nRequires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.\n"};

int64_t ParseISO8601DateTime(const std::string& str)
{
static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0);
static const std::locale loc(std::locale::classic(),
new boost::posix_time::time_input_facet("%Y-%m-%dT%H:%M:%SZ"));
std::istringstream iss(str);
iss.imbue(loc);
boost::posix_time::ptime ptime(boost::date_time::not_a_date_time);
iss >> ptime;
if (ptime.is_not_a_date_time() || epoch > ptime)
return 0;
return (ptime - epoch).total_seconds();
}

bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) {
bool can_avoid_reuse = wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool();
Expand Down
3 changes: 1 addition & 2 deletions src/wallet/rpc/util.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2017-2022 The Bitcoin Core developers
// Copyright (c) 2017-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand Down Expand Up @@ -51,7 +51,6 @@ std::string LabelFromValue(const UniValue& value);
void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry);

void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error);
int64_t ParseISO8601DateTime(const std::string& str);
void AppendLastProcessedBlock(UniValue& entry, const CWallet& wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
} // namespace wallet

Expand Down
1 change: 0 additions & 1 deletion src/wallet/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ target_sources(test_bitcoin
init_tests.cpp
ismine_tests.cpp
psbt_wallet_tests.cpp
rpc_util_tests.cpp
scriptpubkeyman_tests.cpp
spend_tests.cpp
wallet_crypto_tests.cpp
Expand Down
1 change: 0 additions & 1 deletion src/wallet/test/fuzz/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ target_sources(fuzz
crypter.cpp
fees.cpp
$<$<BOOL:${USE_SQLITE}>:${CMAKE_CURRENT_LIST_DIR}/notifications.cpp>
parse_iso8601.cpp
$<$<BOOL:${USE_SQLITE}>:${CMAKE_CURRENT_LIST_DIR}/scriptpubkeyman.cpp>
spend.cpp
wallet_bdb_parser.cpp
Expand Down
24 changes: 0 additions & 24 deletions src/wallet/test/rpc_util_tests.cpp

This file was deleted.

2 changes: 1 addition & 1 deletion test/lint/lint-includes.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
EXCLUDED_DIRS = ["contrib/devtools/bitcoin-tidy/",
] + SHARED_EXCLUDED_SUBTREES

EXPECTED_BOOST_INCLUDES = ["boost/date_time/posix_time/posix_time.hpp",
EXPECTED_BOOST_INCLUDES = [
"boost/multi_index/detail/hash_index_iterator.hpp",
"boost/multi_index/hashed_index.hpp",
"boost/multi_index/identity.hpp",
Expand Down
1 change: 0 additions & 1 deletion vcpkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"$schema": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
"builtin-baseline": "c82f74667287d3dc386bce81e44964370c91a289",
"dependencies": [
"boost-date-time",
"boost-multi-index",
"boost-signals2",
"libevent"
Expand Down

0 comments on commit ae69fc3

Please sign in to comment.