Merge #20216: wallet: fix buffer over-read in SQLite file magic check

56a461f72796ca60de28e78f144741eb1a4f5213 wallet: fix buffer over-read in SQLite file magic check (Sebastian Falbesoner)

Pull request description:

  Looking at our new SQLite database code, I noticed that there is a potential problem in the method `IsSQLiteFile()`:  If there is no terminating zero within the first 16 bytes of the file, the `magic` buffer would be over-read in the `std::string` constructor for `magic_str`. Fixed by using the "from buffer" variant of the string ctor (that also takes a size) rather than the "from c-string" variant (see http://www.cplusplus.com/reference/string/string/string/).

  The behaviour can be reproduced by the following steps:
  * Creating a file of at least 512 bytes in size (to pass the minimum size check) that doesn't contain zero bytes in the magic area, e.g. simply:
  `$ python3 -c "print('A'*512)" > /tmp/corrupt_wallet`
  * Showing content and size of the `magic_str` string in case the magic check fails
  * Create a simple unit test that simply calls `IsSQLiteFile` with the corrupt wallet file
  * Run the unit test and see the random gibberish output of `magic_str` after 16 `A`s :-)

  Or, TLDR variant, just get the branch https://github.com/theStack/bitcoin/tree/reproduce_sqlite_magic_overread, compile unit Tests and run the script `./reproduce_sqlite_magic_overread.sh`.

  Note that this is the minimal diff, probably it would be better to avoid `std::string` at all in this case and just use `memcmp`, strings that include null bytes are pretty confusing.

ACKs for top commit:
  promag:
    Code review ACK 56a461f72796ca60de28e78f144741eb1a4f5213.
  practicalswift:
    ACK 56a461f72796ca60de28e78f144741eb1a4f5213: patch looks correct
  achow101:
    ACK 56a461f72796ca60de28e78f144741eb1a4f5213

Tree-SHA512: a7aadd4d38eb92337e6281df2980f4bde744dbb6cf112b9cd0f2cab8772730e302db9123a8fe7ca4e7e844c47e68957487adb2bed4518c40b4bed6a69d7922b4

1 files changed, 2 insertions, 2 deletions

diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index 02a161ecbd..6d2fdbe58b 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -619,8 +619,8 @@ bool IsSQLiteFile(const fs::path& path)
     file.close();
 
     // Check the magic, see https://sqlite.org/fileformat2.html
-    std::string magic_str(magic);
-    if (magic_str != std::string("SQLite format 3")) {
+    std::string magic_str(magic, 16);
+    if (magic_str != std::string("SQLite format 3", 16)) {
         return false;
     }