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

56a461f727 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 56a461f727.
  practicalswift:
    ACK 56a461f727: patch looks correct
  achow101:
    ACK 56a461f727

Tree-SHA512: a7aadd4d38eb92337e6281df2980f4bde744dbb6cf112b9cd0f2cab8772730e302db9123a8fe7ca4e7e844c47e68957487adb2bed4518c40b4bed6a69d7922b4
This commit is contained in:
fanquake 2020-10-23 08:49:17 +08:00
commit 9af7c1993b
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1

View file

@ -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;
}