trackslash
VAULT-90 P2

Check each QR code's details more strictly when importing

0
All issues

Description

What

QR code import (Backups → Restore → QR Code Import) should check each code's group details before it uses them. A code that doesn't fit should show the scanner's existing invalid-code state.

The code is in VaultBackup/Export/DataShardDecoder.swift (add(shardData:)), with DataShard.GroupInfo (id, number, totalNumber). BackupImportScanningHandler already turns an error thrown from add into .continueScanning(.invalidCode), so the fix only needs the decoder to throw.

  • Check each code's own fields before using them. totalNumber must be between 1 and an upper limit comfortably above the largest real export. Work that limit out from the biggest backup Vault can make, and write down how. number must be in 0 ..< totalNumber.
  • Check each code against the codes already scanned. Today only the group id is compared. Also require the same totalNumber as the first code.
  • Don't let anything read from a code decide how much memory or time the decoder uses until it has passed these checks.
  • Add new AddShardError cases for these failures. Leave canIgnoreError false for them, so they show as invalid rather than being silently skipped.

Tests

  • DataShardDecoderTests: one test for each rejected case. After a rejected code, the decoder's state hasn't changed, and a valid set of codes still decodes.
  • BackupImportScanningHandlerTests: a rejected code gives .invalidCode, and scanning carries on.
  • VAULT-87's fuzz test for the QR shard decoder covers this decoder too.

Disclosure

This follows VAULT-82's rule: a neutral PR title and commit message, and no detail beyond this ticket until the fix ships.

Related: VAULT-82 (security review), VAULT-87 (restore tests, hostile input).

GitHub

0

No branches or pull requests linked.

Comments

2
Bradley

PR: https://github.com/badbundle/vault-app/pull/673. It's validated (Validate (local) is green) and waiting to be merged.

One change from the ticket: a code whose total doesn't match the first code's is treated as a code from another backup (.inconsistentGroup), and the scan skips it as before, rather than showing it as invalid. Group IDs are a random 16 bits, and debug builds give every PDF group ID 0, so a mismatch like that really is a code from another backup. Totals and numbers that are out of range throw new errors that can't be ignored, so those codes show as invalid.