aboutsummaryrefslogtreecommitdiff
path: root/src/util
AgeCommit message (Collapse)Author
2022-04-21Disallow more unsafe string->path conversions allowed by path append operatorsRyan Ofsky
Add more fs::path operator/ and operator+ overloads to prevent unsafe string->path conversions on Windows that would cause strings to be decoded according to the current Windows locale & code page instead of the correct string encoding. Update application code to deal with loss of implicit string->path conversions by calling fs::u8path or fs::PathFromString explicitly, or by just changing variable types from std::string to fs::path to avoid conversions altoghther, or make them happen earlier. In all cases, there's no change in behavior either (1) because strings only contained ASCII characters and would be decoded the same regardless of what encoding was used, or (2) because of the 1:1 mapping between paths and strings using the PathToString and PathFromString functions. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2022-04-21Merge bitcoin/bitcoin#24213: refactor: use Span in random.*laanwj
3ae7791bcaa88f5c68592673b8926ee807242ce7 refactor: use Span in random.* (pasta) Pull request description: ~This PR does two things~ 1. use a Span<unsigned char> for GetRandBytes and GetStrongRandBytes ~2. make GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided. This simplifies a lot of code from `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()`~ MarcoFalke this was inspired by your comment here: https://github.com/bitcoin/bitcoin/pull/24185#issuecomment-1025514263 about using Span, so hopefully I'll be able to get this PR done and merged 😂 ~Also, if requested I could revert the `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()` related changes if it ends up causing too many conflicts~ ACKs for top commit: laanwj: Thank you! Code review re-ACK 3ae7791bcaa88f5c68592673b8926ee807242ce7 Tree-SHA512: 12375a83b68b288916ba0de81cfcab4aac14389a66a36811ae850427435eb67dd55e47df9ac3ec47db4e214f4330139e548bec815fff8a3f571484ea558dca79
2022-04-21util, refactor: Drop explicit conversion to fs::pathHennadii Stepanov
Removes unhelpful noise/verbosity. See: https://github.com/bitcoin/bitcoin/pull/24306#discussion_r809363741
2022-04-21util, refactor: Use GetPathArg to read "-conf" valueHennadii Stepanov
Also "includeconf" values been normalized.
2022-04-19Merge bitcoin/bitcoin#24357: refactor: make setsockopt() and ↵laanwj
SetSocketNoDelay() mockable/testable a2c4a7acd1dfb2fb7e3c9dac6b3d8c9354b2e0a6 net: use Sock::SetSockOpt() instead of standalone SetSocketNoDelay() (Vasil Dimov) d65b6c3fb9cdd41fa53bc76a7b8f49aaa089b0bc net: use Sock::SetSockOpt() instead of setsockopt() (Vasil Dimov) 184e56d6683d05fc84f5153cfff83a2e32883556 net: add new method Sock::SetSockOpt() that wraps setsockopt() (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ Add a `virtual` (thus mockable) method `Sock::SetSockOpt()` that wraps the system `setsockopt()`. Convert the standalone `SetSocketNoDelay()` function to a `virtual` (thus mockable) method `Sock::SetNoDelay()`. This will help avoid syscalls during testing and to mock them to return whatever is suitable for the tests. ACKs for top commit: laanwj: Code review ACK a2c4a7acd1dfb2fb7e3c9dac6b3d8c9354b2e0a6 jonatack: ACK a2c4a7acd1dfb2fb7e3c9dac6b3d8c9354b2e0a6 change since last review is folding `Sock::SetNoDelay()` into the callers Tree-SHA512: 3e2b016c1e4128317a28c17dc9b30472949e1ac3b071b2697c6d30cbcc830df1ee4392a4e23b2ea1ab4e3fb0f59ef450e2a4f3c1df3d8c803dd081652b6c7387
2022-04-17util: optimizes HexStrMartin Leitner-Ankerl
In my benchmark, this rewrite improves runtime 27% (g++) to 46% (clang++) for the benchmark `HexStrBench`: g++ 11.2.0 | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 0.94 | 1,061,381,310.36 | 0.7% | 12.00 | 3.01 | 3.990 | 1.00 | 0.0% | 0.01 | `HexStrBench` master | 0.68 | 1,465,366,544.25 | 1.7% | 6.00 | 2.16 | 2.778 | 1.00 | 0.0% | 0.01 | `HexStrBench` branch clang++ 13.0.1 | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 0.80 | 1,244,713,415.92 | 0.9% | 10.00 | 2.56 | 3.913 | 0.50 | 0.0% | 0.01 | `HexStrBench` master | 0.43 | 2,324,188,940.72 | 0.2% | 4.00 | 1.37 | 2.914 | 0.25 | 0.0% | 0.01 | `HexStrBench` branch Note that the idea for this change comes from denis2342 in PR 23364. This is a rewrite so no unaligned accesses occur. Also, the lookup table is now calculated at compile time, which hopefully makes the code a bit easier to review.
2022-04-16util/check: Add CHECK_NONFATAL identity function, NONFATAL_UNREACHABLE AND ↵Aurèle Oulès
UNREACHABLE macros
2022-04-16Simplify GetTimeMarcoFalke
2022-04-15net: add new method Sock::SetSockOpt() that wraps setsockopt()Vasil Dimov
This will help to increase `Sock` usage and make more code mockable.
2022-04-11refactor: introduce single-separator split helper `SplitString`Sebastian Falbesoner
This helper uses spanparsing::Split internally and enables to replace all calls to boost::split where only a single separator is passed. Co-authored-by: Martin Ankerl <Martin.Ankerl@gmail.com> Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2022-04-05Remove gui-only syscallsMarcoFalke
* Revert "util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem)" This reverts commit f05a4cdf5a0363e1c12f00c034afb60e7ea0c775. * Revert "util: add linkat to syscall sandbox (AllowFileSystem)" This reverts commit 9809db3577f0fa618bea42635b1581e628a30395.
2022-04-05init: Disable syscall sandbox in the bitcoin-qt processMarcoFalke
2022-04-13Merge bitcoin/bitcoin#24355: util, refactor: Add UNIQUE_NAME helper macrolaanwj
1633f5ec8846408182cceb60dc88f022635f4002 util, refactor: Add UNIQUE_NAME helper macro (Hennadii Stepanov) Pull request description: This PR replaces repetitive code with a helper macro. ACKs for top commit: laanwj: Tested ACK 1633f5ec8846408182cceb60dc88f022635f4002 Tree-SHA512: 5f04e472c5f3184c0a9df75395377c6744bfb2cd8f95f8427c1c5e20daa7d6a9b29e45424b88391fc6326d365907a750ab50fda534b49d1df80dccf0e18467a4
2022-04-04Merge bitcoin/bitcoin#24729: util/check: avoid unused parameter warningsMarcoFalke
0add4dbadbc972933b0c99813a155a4ed4852975 util/check: avoid unused parameter warnings (Anthony Towns) Pull request description: Add `[[maybe_unused]]` annotations to avoid warnings from gcc 9.4 and earlier which don't analyse `if constexpr` properly. ACKs for top commit: MarcoFalke: review ACK 0add4dbadbc972933b0c99813a155a4ed4852975 jonatack: ACK 0add4dbadbc972933b0c99813a155a4ed4852975 review and debug build on clang 15 shaavan: ACK 0add4dbadbc972933b0c99813a155a4ed4852975 Tree-SHA512: 3ba490d74d91692c1d22b927da43a130c92cd6a20ed168573e4fbe1f4675fef7e05ebf0b11f2bbd15da3c606fea1f8e6403cfca347009b8b6acc1e77bbee9963
2022-04-01refactor: add missing std:: includes to threadnames.cppfanquake
2022-04-01doc: remove incorrect mention of PR_GET_NAMEfanquake
By removing the whole comment. These #include // For comments are near impossible to maintain, pollute diffs, and generally don't add a lot of value.
2022-04-01util/check: avoid unused parameter warningsAnthony Towns
2022-03-31Merge bitcoin/bitcoin#24714: util/check: Don't use a lambda for Assert/AssumeMarcoFalke
2ef47ba6c57a12840499a13908ab61aefca6cb55 util/check: stop using lambda for Assert/Assume (Anthony Towns) 7c9fe25c16d48b53a61fa2f6ff77eaf8820cb1f6 wallet: move Assert() check into constructor (Anthony Towns) Pull request description: Using a lambda creates a couple of odd namespacing issues, in particular making clang's thread safety analysis less helpful, and confusing gcc when calling member functions. Fix this by not using a lambda. Fixes #21596 Fixes #24654 ACKs for top commit: MarcoFalke: ACK 2ef47ba6c57a12840499a13908ab61aefca6cb55 🚢 jonatack: Tested re-ACK 2ef47ba6c57a12840499a13908ab61aefca6cb55 Tree-SHA512: 4bdbf3215f3d14472df0552362c5eebe8b7eea2d0928a8a41109edd4e0c5f95de6f8220eb2fee8506874e352c003907faf5ef344174795939306a618157b1bae
2022-03-30util/check: stop using lambda for Assert/AssumeAnthony Towns
2022-03-30Merge bitcoin/bitcoin#24690: util: Add inotify_rm_watch to syscall sandbox ↵fanquake
(AllowFileSystem) f05a4cdf5a0363e1c12f00c034afb60e7ea0c775 util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov) Pull request description: This PR fixes the current master (3297f5c11c72dd83479ff8335e047555e3f8cb3b) when running `bitcoin-qt` on Ubuntu 22.04 and quitting: ``` $ ./src/qt/bitcoin-qt -signet -sandbox=log-and-abort Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway. ERROR: The syscall "inotify_rm_watch" (syscall number 255) is not allowed by the syscall sandbox in thread "main". Please report. terminate called without an active exception Aborted (core dumped) ``` Also see https://github.com/bitcoin/bitcoin/pull/24659#discussion_r835747166 ACKs for top commit: fanquake: ACK f05a4cdf5a0363e1c12f00c034afb60e7ea0c775 - checked that qt is using this in it's filesystem watcher code. Tree-SHA512: 9c7920a25422cd3a040bc1cbc487c12c3dc2b91358c3757f1030d6a1ff12c18c688a8e5b7466f683da88a5e4f5f15d442975660022d706e47021253c24c58f4a
2022-03-29Merge bitcoin/bitcoin#24523: build: Fix Boost.Process test for Boost 1.78laanwj
532c64a7264dd3c7329e8839547837c57da7dbe8 build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov) Pull request description: Rebased #24415 with Luke's suggestion. Fixes #24413. ACKs for top commit: hebasto: ACK 532c64a7264dd3c7329e8839547837c57da7dbe8, tested on Mac mini (M1, 2020) + macOS Monterey 12.3 (21E230). Tree-SHA512: 74f779695f6bbc45a2b7341a1402f747cc0d433d74825c7196cb9f156db0c0299895365f01665bd0bff12a8ebb5ea33a29b9a52f5eac0007ec35d1dca6544705
2022-03-28util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem)Hennadii Stepanov
2022-03-24util: add linkat to syscall sandbox (AllowFileSystem)fanquake
2022-03-23refactor: use Span in random.*pasta
2022-03-21build: Fix Boost.Process test for Boost 1.78Hennadii Stepanov
2022-03-10Merge bitcoin/bitcoin#24371: util: Fix `ReadBinaryFile` reading beyond maxsizeMarcoFalke
a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 util: Fix ReadBinaryFile reading beyond maxsize (klementtan) Pull request description: Currently `ReadBinaryFile` will read beyond `maxsize` if `maxsize` is not a multiple of `128` (size of buffer) This is due to `fread` being called with `count = 128` instead of `count = min(128, maxsize - retval.size()` at every iteration The following unit test will fail: ```cpp BOOST_AUTO_TEST_CASE(util_ReadWriteFile) { fs::path tmpfolder = m_args.GetDataDirBase(); fs::path tmpfile = tmpfolder / "read_binary.dat"; std::string expected_text(300,'c'); { std::ofstream file{tmpfile}; file << expected_text; } { // read half the contents in file auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2); BOOST_CHECK_EQUAL(text.size(), 150); } } ``` Error: ``` test/util_tests.cpp:2593: error: in "util_tests/util_ReadWriteFile": check text.size() == 150 has failed [256 != 150] ``` ACKs for top commit: laanwj: Code review ACK a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 theStack: Code-review ACK a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 Tree-SHA512: 752eebe58bc2102dec199b6775f8c3304d899f0ce36d6a022a58e27b076ba945ccd572858b19137b769effd8c6de73a9277f641be24dfb17657fb7173ea0eda0
2022-03-07qt: Avoid crash on startup if int specified in settings.jsonRyan Ofsky
Fix GUI startup crash reported by Rspigler in https://github.com/bitcoin/bitcoin/issues/24457 that happens if settings.json contains an integer value for any of the configuration options which GUI settings can currently clash with (-dbcache, -par, -spendzeroconfchange, -signer, -upnp, -natpmp, -listen, -server, -proxy, -proxy, -onion, -onion, -lang, and -prune). Fix is a one-line change in ArgsManager::GetArg.
2022-03-02util, refactor: Use GetPathArg to read "-settings" valueRyan Ofsky
Take advantage of GetPathArg to simplify code slightly.
2022-03-02util: Add GetPathArg default path argumentRyan Ofsky
Let GetPathArg method be used more places for path arguments that have default values, like "-settings" and BITCOIN_SETTINGS_FILENAME in the next commit. Also: - Fix negated argument handling. Return path{} not path{"0"} when path argument is negated. - Add new tests for default and negated cases - Move GetPathArg() method declaration next to GetArg() declarations. The two methods are close substitutes for each other, so this should help keep them consistent and make them more discoverable.
2022-02-22refactor: shift CopyrightHolders() and LicenseInfo() to clientversion.cppfanquake
2022-02-21util: Add SaturatingAdd helperMarcoFalke
2022-02-21Merge bitcoin/bitcoin#24231: streams: Fix read-past-the-end and integer ↵MarcoFalke
overflows fa1b89a6bdbab50bdb0504782afd4bb3375d1b57 scripted-diff: Rename nReadPos to m_read_pos in streams.h (MarcoFalke) fa56c79df91e5d87533af38b64f4f4148a48a276 Make CDataStream work properly on 64-bit systems (MarcoFalke) fab02f799194c75af7def3a2ab45c443b75de230 streams: Fix read-past-the-end and integer overflows (MarcoFalke) Pull request description: This is a follow-up to commit e26b62093ae21e89ed7d36a24a6b863f38ec631d with the following fixes: * Fix unsigned integer overflow in `ignore()`, when `nReadPos` wraps. * Fix unsigned integer overflow in `read()`, when `nReadPos` wraps. * Fix read-past-the-end in `read()`, when `nReadPos` wraps. This shouldn't be remote-exploitable, because it requires a stream of more than 1GB of size. However, it might be exploitable if the attacker controls the datadir (I haven't checked). A unit test for the overflow in `ignore()` looks like following. It is left as an excercise to the reader to replace `foo.ignore(7)` with the appropriate call to `read()` to reproduce the overflow and read-error in `read()`. ```diff diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 922fd8e513..ec6ea93919 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -534,6 +534,20 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) } catch (const std::ios_base::failure&) { } + CDataStream foo{0, 0}; + auto size{std::numeric_limits<uint32_t>::max()}; + foo.resize(size); + BOOST_CHECK_EQUAL(foo.size(), size); + foo.ignore(std::numeric_limits<int32_t>::max()); + size -= std::numeric_limits<int32_t>::max(); + BOOST_CHECK_EQUAL(foo.size(), size); + foo.ignore(std::numeric_limits<int32_t>::max()); + size -= std::numeric_limits<int32_t>::max(); + BOOST_CHECK_EQUAL(foo.size(), size); + BOOST_CHECK_EQUAL(foo.size(), 1); + foo.ignore(7); // Should overflow, as the size is only 1 + BOOST_CHECK_EQUAL(foo.size(), uint32_t(1 - 7)); + // Very large scriptPubKey (3*10^9 bytes) past the end of the stream CDataStream tmp(SER_DISK, CLIENT_VERSION); uint64_t x = 3000000000ULL; ``` ACKs for top commit: klementtan: Code Review ACK fa1b89a6bdbab50bdb0504782afd4bb3375d1b57: Tree-SHA512: 67f0a1baafe88eaf1dc844ac55b638d5cf168a18c945e3bf7a2cb03c9a5976674a8e3af2487d8a2c3eae21e5c0e7a519c8b16ee7f104934442e2769d100660e9
2022-02-19util: Fix ReadBinaryFile reading beyond maxsizeklementtan
2022-02-17util: Add missing rseq to syscall sandboxlaanwj
Fixes #24368.
2022-02-17Merge bitcoin/bitcoin#24331: util: Revert back `MoveFileExW` call for MinGW-w64laanwj
dc01cbc538765f64326bca30952c83e3862d0d54 test: Add fs_tests/rename unit test (Hennadii Stepanov) d4999d40b9bd04dc20111aaaa6ed2d3db1a5caf9 util: Revert back MoveFileExW call for MinGW-w64 (Hennadii Stepanov) Pull request description: Unfortunately, bitcoin/bitcoin#24308 introduced a [regression](https://github.com/bitcoin/bitcoin/pull/24308#issuecomment-1037259386) for mingw builds. The root of the problem is a broken implementation of [`std::filesystem::rename`](https://en.cppreference.com/w/cpp/filesystem/rename). In particular, the expected behavior > If `old_p` is a non-directory file, then `new_p` must be ... existing non-directory file: `new_p` _is first deleted_... fails with the "File exists" error. This PR reverts back the `MoveFileExW` call, and adds the [suggested](https://github.com/bitcoin/bitcoin/pull/24308#pullrequestreview-878832906) unit test. ACKs for top commit: vasild: ACK dc01cbc538765f64326bca30952c83e3862d0d54 Tree-SHA512: c8e5a98844cfa32bec0ad67a1aaa58fe2efd0c5474d3e83490211985b110f83245758a742dcaa0a933a192ab66a7f11807e0c53ae69260b7dd02fc99f6d03849
2022-02-16util, refactor: Add UNIQUE_NAME helper macroHennadii Stepanov
This change replaces repetitive code with a helper macro.
2022-02-14util: Add missing unlinkat to syscall sandboxMarcoFalke
2022-02-14util: Revert back MoveFileExW call for MinGW-w64Hennadii Stepanov
2022-02-10util: use stronger-guarantee rename methodVasil Dimov
Use std::filesystem::rename() instead of std::rename(). We rely on the destination to be overwritten if it exists, but std::rename()'s behavior is implementation-defined in this case.
2022-02-10Merge bitcoin/bitcoin#24297: Fix unintended unsigned integer overflow in ↵fanquake
strencodings fac9fe5d051264fcd16e8e36d30f28c05c999837 Fix unintended unsigned integer overflow in strencodings (MarcoFalke) Pull request description: This fixes two issues for strings that start with a colon and only have one colon: * `fMultiColon` is incorrectly set to `true` * There is an unsigned integer overflow `colon - 1` (`0 - 1`) Neither issue matters, as the result is discarded. Though, it makes sense to still fix the issue for clarity and to avoid sanitizer issues in the function. ACKs for top commit: laanwj: Code review ACK fac9fe5d051264fcd16e8e36d30f28c05c999837 shaavan: Code Review ACK fac9fe5d051264fcd16e8e36d30f28c05c999837 Tree-SHA512: e71c21a0b617abf241e561ce6b90b963e2d5e2f77bd9547ce47209a1a94b454384391f86ef5d35fedd4f4df19add3896bb3d61fed396ebba8e864e3eeb75ed59
2022-02-10Merge bitcoin/bitcoin#24298: fuzz: Avoid unsigned integer overflow in ↵fanquake
FormatParagraph fa2f7d005932bff9b7d27744ae517b9e7910df8d fuzz: Avoid unsigned integer overflow in FormatParagraph (MarcoFalke) Pull request description: `FormatParagraph` is only ever called with compile time constant arguments, so I don't see the need for fuzzing it. Though, keep it for now, but avoid the unsigned integer overflow with this patch. ACKs for top commit: laanwj: Code review ACK fa2f7d005932bff9b7d27744ae517b9e7910df8d Tree-SHA512: 01fc64a9ef73c183921ca1b0cd8db9514c0a242e3acf215a3393f383ae129e01625ebb16eaf9cb86370eda62d0145c3dcf8f62e40edf5958abc1f777c5687280
2022-02-09util: Drop no longer needed StripRedundantLastElementsOfPath() functionHennadii Stepanov
2022-02-09Use ArgsManager::GetPathArg() for "-blocksdir" optionHennadii Stepanov
2022-02-09Use ArgsManager::GetPathArg() for "-datadir" optionHennadii Stepanov
2022-02-09util: Add ArgsManager::GetPathArg() functionHennadii Stepanov
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2022-02-09streams: Fix read-past-the-end and integer overflowsMarcoFalke
2022-02-09fuzz: Avoid unsigned integer overflow in FormatParagraphMarcoFalke
2022-02-09Fix unintended unsigned integer overflow in strencodingsMarcoFalke
2022-02-08Merge bitcoin/bitcoin#24266: util: Avoid buggy ↵MarcoFalke
std::filesystem:::create_directories() call b9c113af754540341d9529532fbadb7525168102 util: Avoid buggy std::filesystem:::create_directories() call (Hennadii Stepanov) Pull request description: Compiled with some libstdc++ versions (e.g., on Ubuntu 20.04) [`std::filesystem:::create_directories()`](https://en.cppreference.com/w/cpp/filesystem/create_directory) call [fails](https://github.com/bitcoin/bitcoin/issues/24257#issue-1123753243) to handle symbol links properly. No behavior change in comparison to the [pre-20744](https://github.com/bitcoin/bitcoin/commit/c194293883fbb656779102309b2cb3e60889feff) master branch. Fixes bitcoin/bitcoin#24257. ACKs for top commit: ryanofsky: Code review ACK b9c113af754540341d9529532fbadb7525168102. Nice simplification and fix MarcoFalke: review ACK b9c113af754540341d9529532fbadb7525168102 🐬 Tree-SHA512: 79d940cfc1f68d9b0548fb2ab005e90850b54ac0fb3bb2940afd632d56288d92687579a3176bac3fd0ea3d2dae71e26444f8f7bdb87862414c12866ae5e857c4
2022-02-05util: Avoid buggy std::filesystem:::create_directories() callHennadii Stepanov
Compiled with some libstdc++ versions (e.g., on Ubuntu 20.04) std::filesystem:::create_directories() call fails to handle symbol links properly.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 12:58:38 +0000