Merge PR #478

apps/lightcraft-web/src/backup.rs

@@ -11,8 +11,9 @@
 //!
 //! Restoring never deletes anything: the library files go to a new library folder in browser
 //! storage, originals that aren't stored yet are added (they're named by content, so existing
-//! ones are identical), and only then does a small pointer file ([`ACTIVE_LIBRARY`]) switch to
+//! ones are identical; a restored one must match its content hash, see [`verify_original`]),
-//! the restored library. The library that was there before stays in storage.
+//! and only then does a small pointer file ([`ACTIVE_LIBRARY`]) switch to the restored library.
+//! The library that was there before stays in storage.
 //!
 //! No zip64: a backup holds at most [`MAX_BYTES`] and 65 535 entries.
 
@@ -241,6 +242,16 @@ pub fn verify(entry: &ZipEntry, data: &[u8]) -> Result<(), String> {
     Ok(())
 }
 
+/// Check that an original's bytes are the photo its storage key names: originals are stored
+/// under their content hash, and a restore that kept bytes under another photo's hash would
+/// give that photo's catalog record a different image (the zip checksum can't tell).
+pub fn verify_original(entry: &ZipEntry, hash: &str, data: &[u8]) -> Result<(), String> {
+    if crate::store::content_hash(data) != hash {
+        return Err(format!("{}: the file doesn't match its content hash (damaged or altered backup)", entry.name));
+    }
+    Ok(())
+}
+
 /// A name for the restored library's folder.
 pub fn restored_dir_name(now_ms: f64) -> String {
     format!("library-restored-{}", now_ms.max(0.0) as u64)
@@ -283,6 +294,27 @@ mod tests {
         assert!(find_central(b"not a zip at all, just some text").is_err());
     }
 
+    #[test]
+    fn originals_must_match_their_content_hash() {
+        let red: &[u8] = b"red corner photo";
+        let blue: &[u8] = b"blue corner photo";
+        let hash = crate::store::content_hash(red);
+        let name = original_entry(&hash, "IMG 1.png");
+        let key = restore_key(&name, "d").unwrap();
+        let key_hash = key.strip_prefix("originals/").unwrap();
+        assert_eq!(key_hash, hash);
+        // the entry's own bytes pass; other bytes under the same name (valid zip CRC) don't
+        let zip = build(&[(name.as_str(), blue)]);
+        let tail = &zip[zip.len() - tail_len(zip.len() as u64) as usize..];
+        let (off, size) = find_central(tail).unwrap();
+        let entries = parse_central(&zip[off as usize..(off + size) as usize]).unwrap();
+        let entry = &entries[0];
+        verify(entry, blue).unwrap();
+        let err = verify_original(entry, key_hash, blue).unwrap_err();
+        assert!(err.contains(&name), "{err}");
+        verify_original(entry, key_hash, red).unwrap();
+    }
+
     #[test]
     fn restore_keys_are_safe() {
         assert_eq!(restore_key("library/catalog.snap", "library-restored-1").as_deref(), Some("library-restored-1/catalog.snap"));

apps/lightcraft-web/src/safety.rs

@@ -14,7 +14,7 @@ use wasm_bindgen_futures::JsFuture;
 use crate::backend::Backend;
 use crate::backup::{
     ACTIVE_LIBRARY, README, ZipWriter, data_offset, find_central, original_entry, parse_central, restore_key, restored_dir_name, tail_len,
-    valid_hash, verify,
+    valid_hash, verify, verify_original,
 };
 use crate::files::{Files, LIBRARY_FILES};
 use crate::store::storage_key;
@@ -224,6 +224,9 @@ pub async fn restore(file: web_sys::File, backend: Backend) -> Result<String, St
         let start = data_offset(e, &local)?;
         let data = read_range(&file, start, start + e.size).await?;
         verify(e, &data)?;
+        if let Some(hash) = key.strip_prefix("originals/") {
+            verify_original(e, hash, &data)?;
+        }
         backend.write(&key, &data).await.map_err(|err| format!("storing {}: {err}", e.name))?;
         written += 1;
     }

docs/web.md

@@ -111,9 +111,10 @@ are the same in every version, so serve them with `Cache-Control: no-cache` (not
     (`originals/<hash>/<file name>`, so an unzipped backup is browsable; entries are stored, not
     compressed; at most 4 GB). **File ▸ Restore Library from Backup…** reads such a zip into a
     new library folder (`library-restored-<time>/`; originals already stored are skipped, every
-    entry's checksum is verified) and only then switches to it (`active-library` names the
+    entry's checksum is verified, and each restored original must match its content hash) and
-    folder in use) and reloads: the previous library stays in storage. Both are web-only
+    only then switches to it (`active-library` names the folder in use) and reloads: the previous
-    (`file.backupLibrary`, `file.restoreLibrary`; the desktop library is a folder).
+    library stays in storage. Both are web-only (`file.backupLibrary`, `file.restoreLibrary`; the
+    desktop library is a folder).
   - A failed save (quota exceeded, storage cleared) is not silent: the catalog then refuses new
     writes, so commands report `saved in memory but not written`, the top bar shows the unsaved
     warning (as on the desktop, see `docs/control-protocol.md`), and saving is retried every