fix(lite-backend): durable wallet saves, non-destructive backup recovery

do_save flushed the temp wallet and renamed it over the wallet with no
fsync of the temp file, the .bak or the directory, so after a power cut
the rename could be on disk while the data was not, leaving a torn
wallet. The readers then panicked on malformed bytes instead of
returning an error (the gzip Decoder and WalletTx unwraps in
LightWallet::read, the version asserts in data.rs and walletzkey.rs,
from_utf8/diversifier unwraps, and vec![0; len] on a corrupt length
prefix, which can abort the process outright), so read_from_disk's .bak
fallback was never reached. And the fallback itself was destructive: it
copied the .bak over the wallet before reading it, on ANY error, so the
damaged file was lost, a transient I/O error replaced a good newer
wallet with an older .bak, and a wallet written by a newer app ("Don't
know how to read wallet version") was overwritten too, dropping imported
keys that are not in the seed.

- do_save: sync_all the temp file (removed on error), copy the .bak,
  tighten and sync it through a write handle (FlushFileBuffers needs
  write access on Windows), rename, sync the copy-fallback destination,
  then fsync the directory on unix. No MoveFileEx write-through on
  Windows.
- read_from_disk: each parse runs in read_wallet_file under
  catch_unwind (panic = "unwind" is kept for release builds). The .bak
  is used only when the wallet fails with InvalidData, UnexpectedEof or
  InvalidInput, and it is read first, so a bad backup leaves both files
  as they were. A damaged wallet whose header says encrypted is not
  replaced by a plaintext .bak (only an older build leaves one): the
  open fails with an explanation and nothing changes. Otherwise the
  damaged wallet is moved (or, failing that, copied) to
  <wallet>.dat.corrupt-<unix secs>, with a -N suffix while that name is
  taken (a dangling symlink counts as taken), and is never deleted. The
  recovered wallet is tightened as before.
- read_from_disk_reporting returns the same client plus "recovered from
  backup; damaged file kept as <name>" after a recovery, for the FFI;
  read_from_disk wraps it. Both stages are also written to stderr,
  since the logger is not initialised yet when the wallet is opened.
- Readers return Err: a newer wallet version is ErrorKind::Unsupported
  (was InvalidData), and so is a newer WalletTx or key version;
  Decoder::new and WalletTx::read propagate with ?; a bad Utxo version,
  a negative or non-UTF-8 or non-R Utxo address, non-UTF-8 metadata
  addresses, an invalid diversifier and a viewing key with no default
  address are InvalidData. Every length-prefixed string and address is
  read through read_bytes_bounded (take(len).read_to_end, no cap, std's
  own short-read error), so a corrupt length is never allocated up
  front.

Flagged behaviour changes:
- fsync order: tmp, then .bak, then rename, then the dir (unix); saves
  at exit, encrypt and decrypt still run on the UI thread and get
  slower on slow disks;
- the readers return Err instead of panicking;
- the .bak fallback runs only for corrupt files, and the damaged file
  is kept as .dat.corrupt-<ts> (every copy is kept; one from a
  plaintext era keeps its seed, inside the 0700 dir);
- a wallet with a newer format errors (Unsupported) and is never
  replaced;
- a damaged encrypted wallet whose backup predates encryption is not
  opened.
Wallet format v9 is unchanged; valid files read and write byte for byte
as before. No ABI or symbol change.

Tests: a wallet whose parse panicked recovers from the .bak, the
garbage is kept as .dat.corrupt-<ts> and the report names it, and the
next open reports nothing; a newer-version wallet is Unsupported and
both files are unchanged (this fails when only the version error kind is
reverted); an unreadable (0000) wallet is a PermissionDenied error that
changes nothing either; a corrupt wallet with
a corrupt or no .bak changes nothing; a damaged encrypted wallet with a
pre-encryption plaintext .bak is refused with both files unchanged and
nothing moved aside; a recovery never overwrites an earlier
.dat.corrupt-<ts> or follows a dangling symlink; every truncation of a
wallet and of a WalletTx is an Err, crafted Utxo, metadata, note and key
records are the expected Err kinds, and a u64::MAX length returns at
once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-12 13:22:00 -05:00
co-authored by Claude Opus 5.5
parent 39333f80b7
commit 31304a1a63
6 changed files with 535 additions and 42 deletions
@@ -587,6 +587,12 @@ impl LightClient {
}
pub fn read_from_disk(config: &LightClientConfig) -> io::Result<Self> {
Self::read_from_disk_reporting(config).map(|(lc, _)| lc)
}
/// read_from_disk, plus what the open did to the files (s2-D14): Some("recovered from backup; damaged file kept as
/// <name>") when a corrupt wallet was replaced by its backup, for the FFI to pass on.
pub fn read_from_disk_reporting(config: &LightClientConfig) -> io::Result<(Self, Option<String>)> {
if !config.wallet_exists() {
// Try to recover from backup
let bak_path = config.get_wallet_path().with_extension("dat.bak");
@@ -601,25 +607,43 @@ impl LightClient {
}
}
// Try to open the wallet file; if it fails (corrupted/truncated), try the backup
let wallet = match File::open(config.get_wallet_path())
.and_then(|f| {
let mut file_buffer = BufReader::new(f);
LightWallet::read(&mut file_buffer, config)
}) {
Ok(w) => w,
Err(e) => {
warn!("Failed to read wallet file: {}, trying backup", e);
let bak_path = config.get_wallet_path().with_extension("dat.bak");
if bak_path.exists() {
std::fs::copy(&bak_path, config.get_wallet_path())?;
tighten(&config.get_wallet_path(), false);
let mut file_buffer = BufReader::new(File::open(config.get_wallet_path())?);
LightWallet::read(&mut file_buffer, config)?
} else {
// C244: fall back to the backup ONLY for a corrupt or torn file: a parser error, or a parser panic, which
// read_wallet_file turns into InvalidData. Never for a transient I/O error (a sharing violation, EACCES) or a
// wallet written by a newer app (Unsupported, s2-D4): both would replace good data with an older .bak.
let wallet_path = config.get_wallet_path();
let bak_path = wallet_path.with_extension("dat.bak");
let (wallet, recovered) = match Self::read_wallet_file(&wallet_path, config) {
Ok(w) => (w, None),
Err(e) if matches!(e.kind(), ErrorKind::InvalidData | ErrorKind::UnexpectedEof | ErrorKind::InvalidInput) => {
// The logger is not set up yet (the FFI initialises it after the open), so this goes to stderr.
let _ = writeln!(io::stderr(), "Wallet file unreadable ({}); trying the backup", e);
if !bak_path.exists() {
return Err(e);
}
// Read the backup FIRST: if it is bad too, both files are left exactly as they were.
let w = Self::read_wallet_file(&bak_path, config)?;
// s2-D12: never silently drop wallet encryption. Only a .bak last saved by Lite <= 1.1.1 can predate
// the encryption of its wallet (since s1-D6 the first encrypted save backs up the encrypted file).
if !w.is_encrypted() && wallet_header_encrypted(&wallet_path) == Some(true) {
return Err(io::Error::new(ErrorKind::InvalidData,
"The wallet file is damaged and its backup was made before the wallet was encrypted. \
Nothing was changed. Restore from your seed phrase, or move the damaged wallet file aside \
to open the older, unencrypted backup."));
}
// s2-D13: keep the damaged file, and never overwrite an earlier one: move it aside, or copy it aside
// when it cannot be moved, before the backup replaces it.
let corrupt = Self::unused_corrupt_path(&wallet_path);
if let Err(re) = std::fs::rename(&wallet_path, &corrupt) {
std::fs::copy(&wallet_path, &corrupt).map_err(|ce| io::Error::new(ce.kind(),
format!("Couldn't keep the damaged wallet file: {} / {}", re, ce)))?;
}
std::fs::copy(&bak_path, &wallet_path)?;
tighten(&wallet_path, false);
let name = corrupt.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
let _ = writeln!(io::stderr(), "Wallet restored from backup; damaged file kept as {}", corrupt.display());
(w, Some(format!("recovered from backup; damaged file kept as {}", name)))
}
Err(e) => return Err(e),
};
// C254: a wallet or .bak written before owner-only saves (0644 under the default umask) is healed on every
@@ -643,7 +667,32 @@ impl LightClient {
info!("Read wallet with birthday {}", lc.wallet.read().unwrap().get_first_tx_block());
info!("Created LightClient to {}", &config.server);
Ok(lc)
Ok((lc, recovered))
}
/// C244: one parse of a wallet file. A parser panic on a corrupt file becomes InvalidData (lib/Cargo.toml keeps
/// panic = "unwind" for release builds, so the catch is effective).
fn read_wallet_file(path: &Path, config: &LightClientConfig) -> io::Result<LightWallet> {
let f = File::open(path)?;
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| LightWallet::read(BufReader::new(f), config))) {
Ok(r) => r,
Err(_) => Err(io::Error::new(ErrorKind::InvalidData, "wallet file is corrupt (parser panic)")),
}
}
/// s2-D13: <wallet>.dat.corrupt-<unix secs>, with a -1, -2, ... suffix while that name is taken. Any entry counts
/// as taken, a dangling symlink too (symlink_metadata), so an earlier damaged file is never overwritten and a
/// symlink is never followed.
fn unused_corrupt_path(wallet_path: &Path) -> PathBuf {
let ts = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0);
let taken = |p: &Path| std::fs::symlink_metadata(p).is_ok();
let mut p = wallet_path.with_extension(format!("dat.corrupt-{}", ts));
let mut i: u64 = 1;
while taken(&p) {
p = wallet_path.with_extension(format!("dat.corrupt-{}-{}", ts, i));
i += 1;
}
p
}
pub fn init_logging(&self) -> io::Result<()> {
@@ -869,6 +918,14 @@ impl LightClient {
// Only proceed with rename if the write succeeded
if r.is_ok() {
// C244: make the new bytes durable BEFORE they can replace the old file (FlushFileBuffers on Windows,
// F_FULLFSYNC on macOS). Order: the tmp file, the .bak, the rename, then the directory (unix), so every
// crash point leaves a complete wallet or a complete old wallet.
file_buffer.get_ref().sync_all().map_err(|e| {
let _ = std::fs::remove_file(&tmp_path);
format!("Failed to sync wallet file: {}", e)
})?;
// Create backup of existing wallet (if it exists)
if wallet_path.exists() {
// s1-D6: at the plaintext -> encrypted transition, back up the NEW (encrypted) file, so no
@@ -876,8 +933,12 @@ impl LightClient {
let bak_src: &Path = if wallet.is_encrypted() && wallet_header_encrypted(&wallet_path) == Some(false)
{ tmp_path.as_path() } else { &*wallet_path };
match std::fs::copy(bak_src, &bak_path) {
// fs::copy gives the .bak its source's mode: strip the group/other bits (C254)
Ok(_) => tighten(&bak_path, false),
// fs::copy gives the .bak its source's mode: strip the group/other bits (C254). Then make it
// durable (C244) through a write handle: FlushFileBuffers fails on a read-only one on Windows.
Ok(_) => {
tighten(&bak_path, false);
if let Ok(f) = std::fs::OpenOptions::new().write(true).open(&bak_path) { let _ = f.sync_all(); }
}
Err(e) => {
warn!("Failed to create wallet backup: {}", e);
// Non-fatal: continue with the save
@@ -893,8 +954,17 @@ impl LightClient {
let _ = std::fs::remove_file(&tmp_path);
return Err(format!("Failed to save wallet: {} / {}", e, e2));
}
if let Ok(f) = std::fs::OpenOptions::new().write(true).open(&wallet_path) { let _ = f.sync_all(); }
let _ = std::fs::remove_file(&tmp_path);
}
// C244: the rename is durable once its directory is. Unix only: a directory cannot be fsynced on
// Windows, and the rename is not write-through there (s2-D3).
#[cfg(unix)]
{
if let Some(dir) = wallet_path.parent() {
if let Ok(d) = File::open(dir) { let _ = d.sync_all(); }
}
}
}
}
@@ -2609,4 +2679,203 @@ pub fn test_bad_import() {
}
}
// C244 (s2 tests 1-3, the X5 test for s2-D12 and the s2-D13 no-overwrite test): read_from_disk falls back to the
// .bak only for a corrupt wallet, keeps the damaged file, and changes nothing in every other case.
fn corrupt_files(dir: &std::path::Path) -> Vec<std::path::PathBuf> {
let mut v: Vec<std::path::PathBuf> = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().path())
.filter(|p| p.file_name().unwrap().to_string_lossy().contains(".dat.corrupt-")).collect();
v.sort();
v
}
// A saved wallet and its .bak (the second save makes the .bak).
fn saved_pair(tmp: &TempDir) -> (LightClientConfig, std::path::PathBuf, std::path::PathBuf) {
let config = main_config(tmp);
let wallet = config.get_wallet_path().into_path_buf();
let bak = wallet.with_extension("dat.bak");
let lc = LightClient::new_from_phrase(TEST_SEED.to_string(), &config, 0, 0, false).unwrap();
lc.do_save().unwrap();
lc.do_save().unwrap();
assert!(bak.exists());
(config, wallet, bak)
}
// Version 5 selects the gzip path, and 0xAB is no gzip header: Decoder::new(..).unwrap() (LW:323) panicked on it.
fn gzip_garbage() -> Vec<u8> {
let mut b = 5u64.to_le_bytes().to_vec();
b.extend_from_slice(&[0xAB; 32]);
b
}
fn read_err(config: &LightClientConfig) -> std::io::Error {
match LightClient::read_from_disk(config) {
Ok(_) => panic!("the wallet opened"),
Err(e) => e,
}
}
// s2 C244 test 1: a parse that panicked recovers from the .bak, and the damaged file is kept as .dat.corrupt-<ts>.
#[test]
pub fn test_read_from_disk_recovers_when_parse_would_panic() {
let tmp = TempDir::new("lctest").unwrap();
let (config, wallet, bak) = saved_pair(&tmp);
let bak_bytes = std::fs::read(&bak).unwrap();
std::fs::write(&wallet, gzip_garbage()).unwrap();
let (lc, note) = LightClient::read_from_disk_reporting(&config).unwrap();
assert_eq!(lc.do_seed_phrase().unwrap()["seed"], TEST_SEED.to_string());
let kept = corrupt_files(tmp.path());
assert_eq!(kept.len(), 1, "the damaged file is kept");
assert_eq!(std::fs::read(&kept[0]).unwrap(), gzip_garbage());
assert_eq!(std::fs::read(&wallet).unwrap(), bak_bytes, "the wallet is the backup now");
assert_eq!(std::fs::read(&bak).unwrap(), bak_bytes, "the backup is untouched");
let name = kept[0].file_name().unwrap().to_string_lossy().into_owned();
assert!(name.starts_with("silentdragonxlite-wallet.dat.corrupt-"), "{}", name);
assert_eq!(note, Some(format!("recovered from backup; damaged file kept as {}", name)));
drop(lc);
// The next open is a plain one: nothing to report, nothing moved.
let (lc, note) = LightClient::read_from_disk_reporting(&config).unwrap();
assert_eq!(note, None);
assert_eq!(corrupt_files(tmp.path()), kept);
assert_eq!(lc.do_seed_phrase().unwrap()["seed"], TEST_SEED.to_string());
}
// s2 C244 test 2 (s2-D4): a wallet written by a newer app is an Unsupported error that changes nothing; the older
// .bak never replaces it. A transient I/O error (EACCES here) does not fall back either.
#[test]
pub fn test_read_from_disk_keeps_newer_version_wallet() {
let tmp = TempDir::new("lctest").unwrap();
let (config, wallet, bak) = saved_pair(&tmp);
let bak_bytes = std::fs::read(&bak).unwrap();
let mut newer = u64::MAX.to_le_bytes().to_vec();
newer.extend_from_slice(b"a wallet from a newer app");
std::fs::write(&wallet, &newer).unwrap();
let e = read_err(&config);
assert_eq!(e.kind(), std::io::ErrorKind::Unsupported, "{}", e);
assert!(e.to_string().contains("Don't know how to read wallet version"), "{}", e);
assert_eq!(std::fs::read(&wallet).unwrap(), newer, "the newer wallet was replaced");
assert_eq!(std::fs::read(&bak).unwrap(), bak_bytes, "the backup was changed");
assert!(corrupt_files(tmp.path()).is_empty());
#[cfg(unix)]
{
std::fs::write(&wallet, gzip_garbage()).unwrap();
chmod(&wallet, 0o000);
if std::fs::File::open(&wallet).is_err() { // not when the tests run as root
let e = read_err(&config);
chmod(&wallet, 0o600);
assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied, "{}", e);
assert_eq!(std::fs::read(&wallet).unwrap(), gzip_garbage(), "an unreadable wallet was replaced");
assert_eq!(std::fs::read(&bak).unwrap(), bak_bytes);
assert!(corrupt_files(tmp.path()).is_empty());
}
chmod(&wallet, 0o600);
}
}
// s2 C244 test 3: a corrupt wallet with a corrupt (or no) .bak is an error that leaves the files as they were.
#[test]
pub fn test_read_from_disk_both_bad_leaves_files() {
let tmp = TempDir::new("lctest").unwrap();
let (config, wallet, bak) = saved_pair(&tmp);
let mut main_bad = std::fs::read(&wallet).unwrap();
main_bad.truncate(main_bad.len() / 2);
std::fs::write(&wallet, &main_bad).unwrap();
std::fs::write(&bak, gzip_garbage()).unwrap();
let _ = read_err(&config);
assert_eq!(std::fs::read(&wallet).unwrap(), main_bad, "the wallet was changed");
assert_eq!(std::fs::read(&bak).unwrap(), gzip_garbage(), "the backup was changed");
assert!(corrupt_files(tmp.path()).is_empty());
std::fs::remove_file(&bak).unwrap();
let e = read_err(&config);
assert!(matches!(e.kind(), std::io::ErrorKind::InvalidData | std::io::ErrorKind::UnexpectedEof
| std::io::ErrorKind::InvalidInput), "{:?}", e);
assert_eq!(std::fs::read(&wallet).unwrap(), main_bad);
assert!(!bak.exists());
assert!(corrupt_files(tmp.path()).is_empty());
}
// X5 (s2-D12): a damaged encrypted wallet whose .bak predates the encryption (a .bak last saved by Lite <= 1.1.1;
// s1-D6 no longer writes one, so it is built by hand) is not opened: Err, both files unchanged, nothing kept aside.
#[test]
pub fn test_read_from_disk_refuses_plaintext_bak_for_encrypted_main() {
use crate::lightwallet::LightWallet;
use super::wallet_header_encrypted;
let tmp = TempDir::new("lctest").unwrap();
let config = main_config(&tmp);
let wallet = config.get_wallet_path().into_path_buf();
let bak = wallet.with_extension("dat.bak");
let lc = LightClient::new_from_phrase(TEST_SEED.to_string(), &config, 0, 0, false).unwrap();
lc.do_save().unwrap();
let plaintext = std::fs::read(&wallet).unwrap();
assert_eq!(plaintext[8], 0, "precondition: a plaintext wallet");
lc.wallet.write().unwrap().encrypt("password".to_string()).unwrap();
lc.do_save().unwrap();
drop(lc);
std::fs::write(&bak, &plaintext).unwrap();
let enc = std::fs::read(&wallet).unwrap();
assert_eq!(enc[8], 1, "precondition: an encrypted wallet");
let damaged = enc[..enc.len() / 2].to_vec(); // the 9-byte header is intact
std::fs::write(&wallet, &damaged).unwrap();
assert_eq!(wallet_header_encrypted(&wallet), Some(true));
let from_bak = LightWallet::read(std::io::BufReader::new(std::fs::File::open(&bak).unwrap()), &config).unwrap();
assert!(!from_bak.is_encrypted(), "precondition: the .bak opens, unencrypted");
assert_eq!(from_bak.get_seed_phrase(), TEST_SEED.to_string());
let e = read_err(&config);
assert_eq!(e.kind(), std::io::ErrorKind::InvalidData, "{}", e);
assert!(e.to_string().contains("its backup was made before the wallet was encrypted"), "{}", e);
assert_eq!(std::fs::read(&wallet).unwrap(), damaged, "the damaged encrypted wallet was changed");
assert_eq!(std::fs::read(&bak).unwrap(), plaintext, "the backup was changed");
assert!(corrupt_files(tmp.path()).is_empty(), "a damaged file was moved aside");
}
// s2-D13: a recovery never overwrites an earlier .dat.corrupt-<ts>; it takes the next free -<n> suffix. Any entry
// counts as taken, a dangling symlink too (unix), whose target is never created.
#[test]
pub fn test_recovery_never_overwrites_previous_corrupt_file() {
let tmp = TempDir::new("lctest").unwrap();
let (config, wallet, _bak) = saved_pair(&tmp);
let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_secs();
let mut sentinels = vec![];
for t in now..now + 10 { // the recovery may run a few seconds later
let p = wallet.with_extension(format!("dat.corrupt-{}", t));
let s = format!("an earlier damaged wallet {}", t).into_bytes();
std::fs::write(&p, &s).unwrap();
sentinels.push((p, s));
}
#[cfg(unix)]
let (want_suffix, target) = {
let target = tmp.path().join("symlink-target");
for t in now..now + 10 {
std::os::unix::fs::symlink(&target, wallet.with_extension(format!("dat.corrupt-{}-1", t))).unwrap();
}
("-2", Some(target))
};
#[cfg(not(unix))]
let (want_suffix, target): (&str, Option<std::path::PathBuf>) = ("-1", None);
std::fs::write(&wallet, gzip_garbage()).unwrap();
let (_lc, note) = LightClient::read_from_disk_reporting(&config).unwrap();
for (p, s) in &sentinels {
assert_eq!(&std::fs::read(p).unwrap(), s, "{:?} was overwritten", p);
}
let new: Vec<std::path::PathBuf> = corrupt_files(tmp.path()).into_iter()
.filter(|p| std::fs::symlink_metadata(p).unwrap().file_type().is_file() && !sentinels.iter().any(|(q, _)| q == p))
.collect();
assert_eq!(new.len(), 1, "{:?}", new);
let name = new[0].file_name().unwrap().to_string_lossy().into_owned();
assert!(name.ends_with(want_suffix), "{}", name);
assert_eq!(std::fs::read(&new[0]).unwrap(), gzip_garbage());
assert_eq!(note, Some(format!("recovered from backup; damaged file kept as {}", name)));
if let Some(t) = target {
assert!(!t.exists(), "a dangling symlink was followed");
}
}
}
@@ -309,7 +309,7 @@ impl LightWallet {
if version > LightWallet::serialized_version() {
let e = format!("Don't know how to read wallet version {}. Do you have the latest version?", version);
error!("{}", e);
return Err(io::Error::new(ErrorKind::InvalidData, e));
return Err(io::Error::new(ErrorKind::Unsupported, e));
}
println!("Reading wallet version {}", version);
info!("Reading wallet version {}", version);
@@ -320,7 +320,7 @@ impl LightWallet {
Box::new(inp)
} else {
info!("Reading libflat");
Box::new(Decoder::new(inp).unwrap())
Box::new(Decoder::new(inp)?)
};
let encrypted = if version >= 4 {
@@ -431,7 +431,7 @@ impl LightWallet {
let mut txid_bytes = [0u8; 32];
r.read_exact(&mut txid_bytes)?;
Ok((TxId{0: txid_bytes}, WalletTx::read(r).unwrap()))
Ok((TxId{0: txid_bytes}, WalletTx::read(r)?))
})?;
let txs = txs_tuples.into_iter().collect::<HashMap<TxId, WalletTx>>();
@@ -157,7 +157,9 @@ impl SaplingNoteData {
// to recreate the note
let (value, r) = read_note(&mut reader)?; // TODO: This method is in a different package, because of some fields that are private
let maybe_note = extfvk.fvk.vk.into_payment_address(diversifier, &JUBJUB).unwrap().create_note(value, r, &JUBJUB);
let maybe_note = extfvk.fvk.vk.into_payment_address(diversifier, &JUBJUB)
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "invalid diversifier"))?
.create_note(value, r, &JUBJUB);
let note = match maybe_note {
Some(n) => Ok(n),
@@ -262,13 +264,21 @@ impl Utxo {
pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
let version = reader.read_u64::<LittleEndian>()?;
assert_eq!(version, Utxo::serialized_version());
if version != Utxo::serialized_version() {
return Err(io::Error::new(io::ErrorKind::InvalidData, format!("Unknown Utxo version {}", version)));
}
let address_len = reader.read_i32::<LittleEndian>()?;
let mut address_bytes = vec![0; address_len as usize];
reader.read_exact(&mut address_bytes)?;
let address = String::from_utf8(address_bytes).unwrap();
assert_eq!(address.chars().take(1).collect::<Vec<char>>()[0], 'R');
// C244: a negative i32 would sign-extend to a ~2^64 length.
if address_len < 0 {
return Err(io::Error::new(io::ErrorKind::InvalidData, "negative Utxo address length"));
}
let address_bytes = super::utils::read_bytes_bounded(&mut reader, address_len as u64)?;
let address = String::from_utf8(address_bytes)
.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?;
if !address.starts_with('R') {
return Err(io::Error::new(io::ErrorKind::InvalidData, "Utxo address is not a transparent address"));
}
let mut txid_bytes = [0; 32];
reader.read_exact(&mut txid_bytes)?;
@@ -335,15 +345,16 @@ pub struct OutgoingTxMetadata {
impl OutgoingTxMetadata {
pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
let address_len = reader.read_u64::<LittleEndian>()?;
let mut address_bytes = vec![0; address_len as usize];
reader.read_exact(&mut address_bytes)?;
let address = String::from_utf8(address_bytes).unwrap();
let address_bytes = super::utils::read_bytes_bounded(&mut reader, address_len)?;
let address = String::from_utf8(address_bytes)
.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?;
let value = reader.read_u64::<LittleEndian>()?;
let mut memo_bytes = [0u8; 512];
reader.read_exact(&mut memo_bytes)?;
let memo = Memo::from_bytes(&memo_bytes).unwrap();
let memo = Memo::from_bytes(&memo_bytes)
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "invalid memo"))?;
Ok(OutgoingTxMetadata{
address,
@@ -373,9 +384,9 @@ pub struct IncomingTxMetadata {
impl IncomingTxMetadata {
pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
let address_len = reader.read_u64::<LittleEndian>()?;
let mut address_bytes = vec![0; address_len as usize];
reader.read_exact(&mut address_bytes)?;
let address = String::from_utf8(address_bytes).unwrap();
let address_bytes = super::utils::read_bytes_bounded(&mut reader, address_len)?;
let address = String::from_utf8(address_bytes)
.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?;
let value = reader.read_u64::<LittleEndian>()?;
let incoming_mempool = true;
@@ -383,7 +394,8 @@ impl IncomingTxMetadata {
let mut memo_bytes = [0u8; 512];
reader.read_exact(&mut memo_bytes)?;
let memo = Memo::from_bytes(&memo_bytes).unwrap();
let memo = Memo::from_bytes(&memo_bytes)
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "invalid memo"))?;
Ok(IncomingTxMetadata{
address,
@@ -461,7 +473,10 @@ impl WalletTx {
pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
let version = reader.read_u64::<LittleEndian>()?;
assert!(version <= WalletTx::serialized_version(), "Version mismatch. Please restore with your Seed");
if version > WalletTx::serialized_version() {
return Err(io::Error::new(io::ErrorKind::Unsupported,
"Wallet transaction version is newer than this app. Please update, or restore with your Seed"));
}
let block = reader.read_i32::<LittleEndian>()?;
let datetime = if version >= 4 {
@@ -3793,3 +3793,193 @@ fn test_scan_full_tx_mixed_order_epk_no_panic() {
assert!(wallet.incoming_mempool_txs.read().unwrap().contains_key(&TxId(txid)));
});
}
// C244 Fix 2: the wallet readers return Err on corrupt bytes instead of panicking, and a huge length prefix is never
// allocated (an allocation abort cannot be caught). Each case names the old panic it replaces.
#[test]
fn test_wallet_read_never_panics_on_corrupt_input() {
use std::io::{ErrorKind, Read};
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::time::{Duration, Instant};
use super::data::{OutgoingTxMetadata, IncomingTxMetadata, Utxo};
use super::walletzkey::{WalletTKey, WalletZKey};
use super::utils;
// The reader returns, Ok or Err, without panicking, and promptly.
fn run<T, F: FnOnce() -> std::io::Result<T>>(what: &str, f: F) -> std::io::Result<T> {
let t0 = Instant::now();
let r = catch_unwind(AssertUnwindSafe(f));
assert!(t0.elapsed() < Duration::from_secs(5), "{}: took {:?}", what, t0.elapsed());
match r {
Ok(r) => r,
Err(_) => panic!("{}: the reader panicked", what),
}
}
fn kind<T>(what: &str, r: std::io::Result<T>) -> ErrorKind {
match r {
Ok(_) => panic!("{}: read Ok", what),
Err(e) => e.kind(),
}
}
fn find(hay: &[u8], needle: &[u8]) -> usize {
let at: Vec<usize> = hay.windows(needle.len()).enumerate().filter(|(_, w)| *w == needle).map(|(i, _)| i).collect();
assert_eq!(at.len(), 1, "pattern found {} times", at.len());
at[0]
}
// A wallet with a received note, a utxo, and a spend with change (as in test_serialization).
let secp = Secp256k1::new();
let config = get_test_config();
let wallet = LightWallet::new(None, &config, 0, 0).unwrap();
let mut cb1 = FakeCompactBlock::new(0, BlockHash([0; 32]));
let (nf1, txid1) = cb1.add_tx_paying(wallet.zkeys.read().unwrap()[0].extfvk.clone(), 5);
wallet.scan_block(&cb1.as_bytes()).unwrap();
let pk = PublicKey::from_secret_key(&secp, &wallet.tkeys.read().unwrap()[0].tkey.unwrap());
let taddr = wallet.address_from_sk(&wallet.tkeys.read().unwrap()[0].tkey.unwrap());
let mut tx = FakeTransaction::new_with_txid(txid1);
tx.add_t_output(&pk, 20);
wallet.scan_full_tx(&tx.get_tx(), 0, 0);
let addr2 = ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[0u8; 32])).default_address().unwrap().1;
let mut cb2 = FakeCompactBlock::new(1, cb1.hash());
cb2.add_tx_spending((nf1, 5), wallet.zkeys.read().unwrap()[0].extfvk.clone(), addr2, 2);
wallet.scan_block(&cb2.as_bytes()).unwrap();
assert_eq!(wallet.txs.read().unwrap()[&txid1].utxos.len(), 1);
// A valid wallet reads, and re-writes byte for byte, as before.
let mut full = vec![];
wallet.write(&mut full).unwrap();
let back = run("the wallet", || LightWallet::read(&full[..], &config)).unwrap();
let mut again = vec![];
back.write(&mut again).unwrap();
assert_eq!(again, full);
// The whole-wallet prefix stride: every truncation is an Err (LW:434 WalletTx::read(r).unwrap() panicked when the
// cut fell inside a tx). The birthday is the last field, so no proper prefix reads.
let mut cuts: Vec<usize> = (0..full.len()).step_by(13).collect();
cuts.extend(full.len().saturating_sub(64)..full.len());
for &n in &cuts {
let what = format!("the wallet cut at {} of {}", n, full.len());
assert!(run(&what, || LightWallet::read(&full[..n], &config)).is_err(), "{}", what);
}
// Whole-wallet cases. The WalletTx of txid1: its txid as the map key, then its version word (DATA:464 assert).
let wtx_at = find(&full, &[&txid1.0[..], &5u64.to_le_bytes()[..]].concat()) + 32;
// The utxo: version 1, an i32 address length, the address (DATA:265/268/270/271).
let utxo_at = find(&full, &[&1u64.to_le_bytes()[..], &(taddr.len() as i32).to_le_bytes()[..], taddr.as_bytes()].concat());
// The chain name's u64 length: the file ends with it, the name and the u64 birthday (utils::read_string).
let chain_at = full.len() - 8 - config.chain_name.len() - 8;
assert_eq!(&full[chain_at..chain_at + 8], &(config.chain_name.len() as u64).to_le_bytes()[..]);
for (what, at, bytes, want) in vec![
("a WalletTx of a newer version", wtx_at, 6u64.to_le_bytes().to_vec(), ErrorKind::Unsupported),
("a Utxo of version 2", utxo_at, 2u64.to_le_bytes().to_vec(), ErrorKind::InvalidData),
("a negative Utxo address length", utxo_at + 8, (-1i32).to_le_bytes().to_vec(), ErrorKind::InvalidData),
("an i32::MIN Utxo address length", utxo_at + 8, i32::MIN.to_le_bytes().to_vec(), ErrorKind::InvalidData),
("a Utxo address that is not a t-address", utxo_at + 12, b"X".to_vec(), ErrorKind::InvalidData),
("a Utxo address that is not UTF-8", utxo_at + 12, vec![0xFF], ErrorKind::InvalidData),
("a u64::MAX chain name length", chain_at, u64::MAX.to_le_bytes().to_vec(), ErrorKind::UnexpectedEof),
] {
let mut f = full.clone();
f[at..at + bytes.len()].copy_from_slice(&bytes);
assert_eq!(kind(what, run(what, || LightWallet::read(&f[..], &config))), want, "{}", what);
}
// The record readers on crafted bytes: WalletTx (every truncation, a newer version).
let txs = wallet.txs.read().unwrap();
let wtx = &txs[&txid1];
let mut b = vec![];
wtx.write(&mut b).unwrap();
assert!(run("a WalletTx", || WalletTx::read(&b[..])).is_ok());
for n in (0..b.len()).step_by(7) {
let what = format!("a WalletTx cut at {} of {}", n, b.len());
assert!(run(&what, || WalletTx::read(&b[..n])).is_err(), "{}", what);
}
let mut v = b.clone();
v[..8].copy_from_slice(&6u64.to_le_bytes());
assert_eq!(kind("a newer WalletTx", run("a newer WalletTx", || WalletTx::read(&v[..]))), ErrorKind::Unsupported);
// SaplingNoteData: a diversifier with no payment address (DATA:160 into_payment_address(..).unwrap()).
let nd = &wtx.notes[0];
let mut b = vec![];
nd.write(&mut b).unwrap();
let mut fvk = vec![];
nd.extfvk.write(&mut fvk).unwrap();
let at = 8 + 8 + fvk.len();
assert_eq!(&b[at..at + 11], &nd.diversifier.0[..]);
let bad = (0u8..=255).map(|i| [i, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10])
.find(|d| nd.extfvk.fvk.vk.into_payment_address(zcash_primitives::primitives::Diversifier(*d), &JUBJUB).is_none())
.unwrap();
let mut v = b.clone();
v[at..at + 11].copy_from_slice(&bad);
assert!(run("a note", || SaplingNoteData::read(&b[..])).is_ok());
assert_eq!(kind("an invalid diversifier", run("an invalid diversifier", || SaplingNoteData::read(&v[..]))),
ErrorKind::InvalidData);
// Utxo (DATA:265 version assert, DATA:268 vec![0; address_len as usize], DATA:270 from_utf8 unwrap, DATA:271 [0]).
let mut b = vec![];
wtx.utxos[0].write(&mut b).unwrap();
assert_eq!(run("a Utxo", || Utxo::read(&b[..])).unwrap().address, taddr);
let tail = b[12 + taddr.len()..].to_vec();
let utxo_with = |ver: u64, len: i32, addr: &[u8]| -> Vec<u8> {
[&ver.to_le_bytes()[..], &len.to_le_bytes()[..], addr, &tail[..]].concat()
};
for (what, bytes, want) in vec![
("a Utxo of version 2", utxo_with(2, taddr.len() as i32, taddr.as_bytes()), ErrorKind::InvalidData),
("a Utxo address length of -1", utxo_with(1, -1, taddr.as_bytes()), ErrorKind::InvalidData),
("a Utxo address length of i32::MIN", utxo_with(1, i32::MIN, taddr.as_bytes()), ErrorKind::InvalidData),
("an empty Utxo address", utxo_with(1, 0, b""), ErrorKind::InvalidData),
("a Utxo address that is not UTF-8", utxo_with(1, 2, &[0xC3, 0x28]), ErrorKind::InvalidData),
("a Utxo address length of i32::MAX", utxo_with(1, i32::MAX, taddr.as_bytes()), ErrorKind::UnexpectedEof),
] {
assert_eq!(kind(what, run(what, || Utxo::read(&bytes[..]))), want, "{}", what);
}
// Outgoing/IncomingTxMetadata (DATA:338/376 vec![0; address_len as usize], DATA:340/378 from_utf8 unwrap). Their
// Memo::from_bytes(..).unwrap() (DATA:346/386) cannot fail on 512 bytes; it is an Err now all the same.
let om = OutgoingTxMetadata { address: "zs1test".to_string(), value: 7, memo: Memo::default() };
let mut b = vec![];
om.write(&mut b).unwrap();
let meta_with = |len: u64, addr: &[u8]| -> Vec<u8> { [&len.to_le_bytes()[..], addr, &b[8 + 7..]].concat() };
assert_eq!(run("OutgoingTxMetadata", || OutgoingTxMetadata::read(&b[..])).unwrap().address, "zs1test");
assert_eq!(run("IncomingTxMetadata", || IncomingTxMetadata::read(&b[..])).unwrap().address, "zs1test");
for (what, bytes, want) in vec![
("a u64::MAX metadata address length", meta_with(u64::MAX, b"zs1test"), ErrorKind::UnexpectedEof),
("a metadata address that is not UTF-8", meta_with(2, &[0xC3, 0x28]), ErrorKind::InvalidData),
] {
assert_eq!(kind(what, run(what, || OutgoingTxMetadata::read(&bytes[..]))), want, "outgoing: {}", what);
assert_eq!(kind(what, run(what, || IncomingTxMetadata::read(&bytes[..]))), want, "incoming: {}", what);
}
// WalletTKey / WalletZKey (WZK:69/275 version asserts); the t-key's address string (utils::read_string).
let mut b = vec![];
wallet.tkeys.read().unwrap()[0].write(&mut b).unwrap();
assert!(run("a WalletTKey", || WalletTKey::read(&b[..])).is_ok());
let mut v = b.clone();
v[0] = 2;
assert_eq!(kind("a newer WalletTKey", run("a newer WalletTKey", || WalletTKey::read(&v[..]))), ErrorKind::Unsupported);
assert_eq!(&b[6..14], &(taddr.len() as u64).to_le_bytes()[..]);
let mut v = b.clone();
v[6..14].copy_from_slice(&u64::MAX.to_le_bytes());
assert_eq!(kind("a u64::MAX t-address length", run("a u64::MAX t-address length", || WalletTKey::read(&v[..]))),
ErrorKind::UnexpectedEof);
let mut b = vec![];
wallet.zkeys.read().unwrap()[0].write(&mut b).unwrap();
assert!(run("a WalletZKey", || WalletZKey::read(&b[..])).is_ok());
let mut v = b.clone();
v[0] = 2;
assert_eq!(kind("a newer WalletZKey", run("a newer WalletZKey", || WalletZKey::read(&v[..]))), ErrorKind::Unsupported);
// read_bytes_bounded: exact lengths read; a short field is std's read_exact error word for word (so a truncated
// file reads exactly as it did); a u64::MAX length is never allocated.
let src = [1u8, 2, 3];
assert_eq!(utils::read_bytes_bounded(&src[..], 3).unwrap(), vec![1, 2, 3]);
assert!(utils::read_bytes_bounded(&src[..], 0).unwrap().is_empty());
let mut four = [0u8; 4];
let std_err = (&src[..]).read_exact(&mut four).unwrap_err();
let ours = run("a short field", || utils::read_bytes_bounded(&src[..], 4)).unwrap_err();
assert_eq!((ours.kind(), ours.to_string()), (std_err.kind(), std_err.to_string()));
let huge = run("a u64::MAX field", || utils::read_bytes_bounded(&src[..], u64::MAX)).unwrap_err();
assert_eq!(huge.kind(), ErrorKind::UnexpectedEof);
let s = [&u64::MAX.to_le_bytes()[..], &b"main"[..]].concat();
assert_eq!(kind("a u64::MAX string length", run("a u64::MAX string length", || utils::read_string(&s[..]))),
ErrorKind::UnexpectedEof);
}
@@ -1,11 +1,23 @@
use std::io::{self, Read, Write};
use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
// C244: read a length-prefixed field of `len` bytes without allocating `len` up front. The buffer grows only with the
// bytes really read, so a corrupt length (u64::MAX) is an UnexpectedEof instead of a capacity-overflow panic or an
// allocation abort (which catch_unwind cannot catch). No cap: every length a valid file holds reads as before. A short
// field gives std's read_exact error word for word, so a truncated file reads exactly as it did.
pub fn read_bytes_bounded<R: Read>(mut r: R, len: u64) -> io::Result<Vec<u8>> {
let mut buf = Vec::new();
r.by_ref().take(len).read_to_end(&mut buf)?;
if buf.len() as u64 != len {
return Err(io::Error::new(io::ErrorKind::UnexpectedEof, "failed to fill whole buffer"));
}
Ok(buf)
}
pub fn read_string<R: Read>(mut reader: R) -> io::Result<String> {
// Strings are written as <littleendian> len + bytes
let str_len = reader.read_u64::<LittleEndian>()?;
let mut str_bytes = vec![0; str_len as usize];
reader.read_exact(&mut str_bytes)?;
let str_bytes = read_bytes_bounded(&mut reader, str_len)?;
let str = String::from_utf8(str_bytes).map_err(|e| {
io::Error::new(io::ErrorKind::InvalidData, e.to_string())
@@ -75,7 +75,10 @@ impl WalletTKey {
pub fn read<R: Read>(mut inp: R) -> io::Result<Self> {
let version = inp.read_u8()?;
assert!(version <= Self::serialized_version());
if version > Self::serialized_version() {
return Err(io::Error::new(ErrorKind::Unsupported,
format!("Wallet key version {} is newer than this app. Please update, or restore with your Seed", version)));
}
let keytype: WalletTKeyType = match inp.read_u32::<LittleEndian>()? {
0 => Ok(WalletTKeyType::HdKey),
@@ -286,7 +289,10 @@ impl WalletZKey {
pub fn read<R: Read>(mut inp: R) -> io::Result<Self> {
let version = inp.read_u8()?;
assert!(version <= Self::serialized_version());
if version > Self::serialized_version() {
return Err(io::Error::new(ErrorKind::Unsupported,
format!("Wallet key version {} is newer than this app. Please update, or restore with your Seed", version)));
}
let keytype: WalletZKeyType = match inp.read_u32::<LittleEndian>()? {
0 => Ok(WalletZKeyType::HdKey),
@@ -299,7 +305,8 @@ impl WalletZKey {
let extsk = Optional::read(&mut inp, |r| ExtendedSpendingKey::read(r))?;
let extfvk = ExtendedFullViewingKey::read(&mut inp)?;
let zaddress = extfvk.default_address().unwrap().1;
let zaddress = extfvk.default_address()
.map_err(|_| io::Error::new(ErrorKind::InvalidData, "invalid viewing key"))?.1;
let hdkey_num = Optional::read(&mut inp, |r| r.read_u32::<LittleEndian>())?;