Return an error, not a panic, on a malformed snapshot database (closes #231)
check / check (push) Successful in 13m39s
check / check (push) Successful in 13m39s
Restore cut chunk hashes from the snapshot database to 16 characters for its error messages, so a shorter hash panicked. Those messages now use shortHash. Under --verify, a file_chunks row with no chunks row was dereferenced, and the chunk size from the database was allocated in one piece, so a negative or huge size panicked. A missing row is now an error, a negative size is rejected, and each chunk is hashed by streaming it from the restored file. A restored file shorter than its chunks now fails verify as a short read instead of an unexpected EOF. Model: opus-5-5
This commit was merged in pull request #249.
This commit is contained in:
+27
-15
@@ -42,6 +42,7 @@ var (
|
||||
errChunkNotInAnyBlob = errors.New("chunk not found in any blob")
|
||||
errBlobIDNotInHashIndex = errors.New("blob id missing from hash index")
|
||||
errShortChunkRead = errors.New("short read")
|
||||
errChunkRowMissing = errors.New("chunk has no row in the chunks table")
|
||||
errRestorePathEscapesTarget = errors.New(
|
||||
"refusing to restore path outside the target directory")
|
||||
errTrailingRestoreData = errors.New(
|
||||
@@ -1267,7 +1268,7 @@ func (s *restoreSession) writeFileChunks(
|
||||
blobChunk, ok := s.chunkToBlobMap[chunkHashStr]
|
||||
if !ok {
|
||||
return bytesWritten, timings, fmt.Errorf(
|
||||
"%w: %s", errChunkNotInAnyBlob, chunkHashStr[:16])
|
||||
"%w: %s", errChunkNotInAnyBlob, shortHash(chunkHashStr))
|
||||
}
|
||||
|
||||
blobHash, ok := s.blobIDToHash[blobChunk.BlobID.String()]
|
||||
@@ -1284,7 +1285,7 @@ func (s *restoreSession) writeFileChunks(
|
||||
if err != nil {
|
||||
return bytesWritten, timings, fmt.Errorf(
|
||||
"reading chunk %s from cached blob %s: %w",
|
||||
fc.ChunkHash[:16], blobHash[:16], err)
|
||||
shortHash(chunkHashStr), shortHash(blobHash), err)
|
||||
}
|
||||
|
||||
t0 = time.Now()
|
||||
@@ -1482,33 +1483,44 @@ func (v *Vaultik) verifyFile(
|
||||
chunk, err := repos.Chunks.GetByHash(ctx, fc.ChunkHash.String())
|
||||
if err != nil {
|
||||
return bytesVerified, fmt.Errorf("getting chunk %s: %w",
|
||||
fc.ChunkHash.String()[:16], err)
|
||||
shortHash(fc.ChunkHash.String()), err)
|
||||
}
|
||||
|
||||
// Read chunk data from file
|
||||
chunkData := make([]byte, chunk.Size)
|
||||
|
||||
n, err := io.ReadFull(f, chunkData)
|
||||
if err != nil {
|
||||
return bytesVerified, fmt.Errorf("reading chunk data: %w", err)
|
||||
if chunk == nil {
|
||||
return bytesVerified, fmt.Errorf("%w: %s",
|
||||
errChunkRowMissing, shortHash(fc.ChunkHash.String()))
|
||||
}
|
||||
|
||||
if int64(n) != chunk.Size {
|
||||
// chunk.Size comes from the snapshot database, which is not
|
||||
// trusted: reject a negative size, and hash the chunk by
|
||||
// streaming it rather than allocating that many bytes.
|
||||
if chunk.Size < 0 {
|
||||
return bytesVerified, fmt.Errorf("%w: chunk %d size %d",
|
||||
errNegativeChunkLength, fc.Idx, chunk.Size)
|
||||
}
|
||||
|
||||
hasher := sha256.New()
|
||||
|
||||
n, err := io.CopyN(hasher, f, chunk.Size)
|
||||
if errors.Is(err, io.EOF) {
|
||||
return bytesVerified, fmt.Errorf("%w: expected %d bytes, got %d",
|
||||
errShortChunkRead, chunk.Size, n)
|
||||
}
|
||||
|
||||
// Calculate hash and compare
|
||||
hash := sha256.Sum256(chunkData)
|
||||
actualHash := hex.EncodeToString(hash[:])
|
||||
if err != nil {
|
||||
return bytesVerified, fmt.Errorf("reading chunk data: %w", err)
|
||||
}
|
||||
|
||||
actualHash := hex.EncodeToString(hasher.Sum(nil))
|
||||
expectedHash := fc.ChunkHash.String()
|
||||
|
||||
if actualHash != expectedHash {
|
||||
return bytesVerified, fmt.Errorf("%w: chunk %d: expected %s, got %s",
|
||||
errChunkHashMismatch, fc.Idx, expectedHash[:16], actualHash[:16])
|
||||
errChunkHashMismatch, fc.Idx,
|
||||
shortHash(expectedHash), shortHash(actualHash))
|
||||
}
|
||||
|
||||
bytesVerified += int64(n)
|
||||
bytesVerified += n
|
||||
}
|
||||
|
||||
// The stored chunks account for the whole file, so the reader must
|
||||
|
||||
Reference in New Issue
Block a user