clawbot
82c51a5337
A blob hash read back from the downloaded snapshot database or the store listing was trusted unchecked. A hostile remote could set a hash such as "aa/../../etc" and have a decrypted blob written outside the cache directory, or feed a short or negative value that panicked a command. blobDiskCache.path now refuses any key with a path separator, and ReadAt rejects a negative offset or length, bounding so a sum cannot overflow past the check. A new isBlobHash helper gates FetchBlob, shallow and deep verify, and restore: buildBlobIndexes rejects every hash from the snapshot database before any fetch. The blobs/ and metadata/ listings skip a non-conforming name, and short-hash prefixes in log and error text go through a panic-safe shortHash helper. Model: opus-4-8
263 lines
8.1 KiB
Go
263 lines
8.1 KiB
Go
package vaultik //nolint:testpackage // drives unexported input validation
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"io"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
"sneak.berlin/go/vaultik/internal/database"
|
|
"sneak.berlin/go/vaultik/internal/log"
|
|
"sneak.berlin/go/vaultik/internal/snapshot"
|
|
"sneak.berlin/go/vaultik/internal/storage"
|
|
"sneak.berlin/go/vaultik/internal/types"
|
|
)
|
|
|
|
// These tests treat every hash, offset and length read back from the
|
|
// destination as hostile. A blob hash comes from the downloaded snapshot
|
|
// database or the store listing, neither of which is authenticated (see
|
|
// https://git.eeqj.de/sneak/vaultik/issues/155), so each is validated
|
|
// before it is used to build a path or size an allocation.
|
|
|
|
// TestBlobCacheRejectsKeyWithSeparator proves the arbitrary-file-write
|
|
// hole is closed: a blob hash that climbs out of the cache directory is
|
|
// refused and nothing is written outside it. This is the exact write the
|
|
// restore path performs, keyed by the hash from the snapshot database.
|
|
func TestBlobCacheRejectsKeyWithSeparator(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
cache, err := newBlobDiskCache(1 << 20)
|
|
require.NoError(t, err)
|
|
|
|
defer func() { _ = cache.Close() }()
|
|
|
|
target := filepath.Join(t.TempDir(), "pwned")
|
|
|
|
// A hash whose relative form escapes the cache directory to target.
|
|
key, err := filepath.Rel(cache.dir, target)
|
|
require.NoError(t, err)
|
|
require.Contains(t, key, "..")
|
|
|
|
err = cache.Put(key, []byte("secret"))
|
|
require.ErrorIs(t, err, errCacheKeyHasSeparator)
|
|
|
|
_, err = cache.PutFromReader(key, strings.NewReader("secret"))
|
|
require.ErrorIs(t, err, errCacheKeyHasSeparator)
|
|
|
|
_, statErr := os.Stat(target)
|
|
require.Truef(t, os.IsNotExist(statErr),
|
|
"cache wrote outside its directory at %s", target)
|
|
}
|
|
|
|
// TestBuildBlobIndexesRejectsHostileHash proves restore refuses a snapshot
|
|
// database whose blob_hash escapes the cache directory. buildBlobIndexes is
|
|
// the first place restore reads these hashes back, and it fails there, before
|
|
// any blob is fetched or written, so a hash containing /../ cannot steer a
|
|
// later write outside the cache directory.
|
|
func TestBuildBlobIndexesRejectsHostileHash(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := context.Background()
|
|
db, err := database.New(ctx, filepath.Join(t.TempDir(), "index.sqlite"))
|
|
require.NoError(t, err)
|
|
|
|
defer func() { _ = db.Close() }()
|
|
|
|
// A blob cache and a target file just outside it. The hostile hash is
|
|
// the relative path from the cache to that target, so an unguarded
|
|
// restore keyed by this hash would write there.
|
|
cache, err := newBlobDiskCache(1 << 20)
|
|
require.NoError(t, err)
|
|
|
|
defer func() { _ = cache.Close() }()
|
|
|
|
target := filepath.Join(t.TempDir(), "pwned")
|
|
hostile, err := filepath.Rel(cache.dir, target)
|
|
require.NoError(t, err)
|
|
require.Contains(t, hostile, "..")
|
|
|
|
repos := database.NewRepositories(db)
|
|
require.NoError(t, repos.Blobs.Create(ctx, nil, &database.Blob{
|
|
ID: types.NewBlobID(),
|
|
Hash: types.BlobHash(hostile),
|
|
CreatedTS: time.Now().UTC(),
|
|
}))
|
|
|
|
v := NewForTesting(nil)
|
|
v.SetContext(ctx)
|
|
|
|
_, _, err = v.buildBlobIndexes(repos)
|
|
require.ErrorIs(t, err, errInvalidBlobHash)
|
|
|
|
_, statErr := os.Stat(target)
|
|
require.Truef(t, os.IsNotExist(statErr),
|
|
"restore wrote outside the cache directory at %s", target)
|
|
}
|
|
|
|
// TestBlobCacheReadAtRejectsBadBounds proves a blob_chunks row cannot
|
|
// drive an out-of-range or negative read. offset/length reach ReadAt
|
|
// straight from the database.
|
|
func TestBlobCacheReadAtRejectsBadBounds(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
cache, err := newBlobDiskCache(1 << 20)
|
|
require.NoError(t, err)
|
|
|
|
defer func() { _ = cache.Close() }()
|
|
|
|
require.NoError(t, cache.Put("blob", make([]byte, 100)))
|
|
|
|
_, err = cache.ReadAt("blob", -1, 10)
|
|
require.ErrorIs(t, err, errCacheNegativeRead)
|
|
|
|
_, err = cache.ReadAt("blob", 0, -1)
|
|
require.ErrorIs(t, err, errCacheNegativeRead)
|
|
|
|
// A length past the end is rejected via the subtraction bound, so a
|
|
// huge offset+length cannot overflow past the check.
|
|
_, err = cache.ReadAt("blob", 50, 60)
|
|
require.ErrorIs(t, err, errCacheReadBeyondBlob)
|
|
}
|
|
|
|
// TestListAllRemoteBlobsSkipsNonConformingName proves a bogus object name
|
|
// under blobs/ (here a three-character name) is skipped rather than
|
|
// entering the blob map, so prune's later hash[:2]/hash[2:4] path build
|
|
// cannot panic on it.
|
|
func TestListAllRemoteBlobsSkipsNonConformingName(t *testing.T) {
|
|
// Initialize the global logger before t.Parallel() so the write lands
|
|
// in the serial phase and cannot race other parallel tests reading it.
|
|
log.Initialize(log.Config{})
|
|
t.Parallel()
|
|
|
|
good := strings.Repeat("a", blobHashHexLen)
|
|
store := &stubLister{objects: []storage.ObjectInfo{
|
|
{Key: "blobs/" + good[:2] + "/" + good[2:4] + "/" + good, Size: 10},
|
|
{Key: "blobs/a/b/c", Size: 3},
|
|
}}
|
|
|
|
v := &Vaultik{Storage: store}
|
|
v.SetContext(context.Background())
|
|
|
|
blobs, err := v.listAllRemoteBlobs()
|
|
require.NoError(t, err)
|
|
require.Contains(t, blobs, good)
|
|
require.NotContains(t, blobs, "c")
|
|
require.Len(t, blobs, 1)
|
|
}
|
|
|
|
// TestVerifyManifestBlobsRejectsShortHash proves a manifest (which is not
|
|
// authenticated) with a short blob hash fails cleanly instead of panicking
|
|
// on blob.Hash[:2].
|
|
func TestVerifyManifestBlobsRejectsShortHash(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
v := &Vaultik{Stdout: io.Discard}
|
|
manifest := &snapshot.Manifest{
|
|
Blobs: []snapshot.BlobInfo{{Hash: "abc", CompressedSize: 1}},
|
|
}
|
|
|
|
verified, missing, mismatched, missingSize, err :=
|
|
v.verifyManifestBlobs(manifest, &VerifyOptions{JSON: true})
|
|
require.ErrorIs(t, err, errInvalidBlobHash)
|
|
require.Zero(t, verified)
|
|
require.Zero(t, missing)
|
|
require.Zero(t, mismatched)
|
|
require.Zero(t, missingSize)
|
|
}
|
|
|
|
// TestVerifyBlobChunksRejectsNegativeLength proves a blob_chunks row with a
|
|
// negative length returns an error rather than reaching make([]byte,
|
|
// length) or streaming an untrusted size.
|
|
func TestVerifyBlobChunksRejectsNegativeLength(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctx := context.Background()
|
|
db, err := database.New(ctx, filepath.Join(t.TempDir(), "index.sqlite"))
|
|
require.NoError(t, err)
|
|
|
|
defer func() { _ = db.Close() }()
|
|
|
|
repos := database.NewRepositories(db)
|
|
blobHash := strings.Repeat("b", blobHashHexLen)
|
|
blob := &database.Blob{
|
|
ID: types.NewBlobID(),
|
|
Hash: types.BlobHash(blobHash),
|
|
CreatedTS: time.Now().UTC(),
|
|
}
|
|
require.NoError(t, repos.Blobs.Create(ctx, nil, blob))
|
|
|
|
chunkHash := strings.Repeat("c", blobHashHexLen)
|
|
require.NoError(t, repos.Chunks.Create(ctx, nil,
|
|
&database.Chunk{ChunkHash: types.ChunkHash(chunkHash), Size: 1024}))
|
|
require.NoError(t, repos.BlobChunks.Create(ctx, nil, &database.BlobChunk{
|
|
BlobID: blob.ID,
|
|
ChunkHash: types.ChunkHash(chunkHash),
|
|
Offset: 0,
|
|
Length: -1,
|
|
}))
|
|
|
|
v := NewForTesting(nil)
|
|
|
|
_, err = v.verifyBlobChunks(db.Conn(), blobHash, strings.NewReader(""))
|
|
require.ErrorIs(t, err, errNegativeChunkLength)
|
|
}
|
|
|
|
// errStubUnused marks a stubLister method a test never exercises.
|
|
var errStubUnused = errors.New("stubLister method not used in test")
|
|
|
|
// stubLister is a storage.Storer whose ListStream yields a fixed set of
|
|
// objects; every other method is unused by the tests here.
|
|
type stubLister struct {
|
|
objects []storage.ObjectInfo
|
|
}
|
|
|
|
func (s *stubLister) ListStream(
|
|
_ context.Context, prefix string,
|
|
) <-chan storage.ObjectInfo {
|
|
ch := make(chan storage.ObjectInfo, len(s.objects))
|
|
for _, o := range s.objects {
|
|
if strings.HasPrefix(o.Key, prefix) {
|
|
ch <- o
|
|
}
|
|
}
|
|
|
|
close(ch)
|
|
|
|
return ch
|
|
}
|
|
|
|
func (s *stubLister) Put(_ context.Context, _ string, _ io.Reader) error {
|
|
return errStubUnused
|
|
}
|
|
|
|
func (s *stubLister) PutWithProgress(
|
|
_ context.Context, _ string, _ io.Reader, _ int64, _ storage.ProgressCallback,
|
|
) error {
|
|
return errStubUnused
|
|
}
|
|
|
|
func (s *stubLister) Get(_ context.Context, _ string) (io.ReadCloser, error) {
|
|
return nil, errStubUnused
|
|
}
|
|
|
|
func (s *stubLister) Stat(_ context.Context, _ string) (*storage.ObjectInfo, error) {
|
|
return nil, errStubUnused
|
|
}
|
|
|
|
func (s *stubLister) Delete(_ context.Context, _ string) error {
|
|
return errStubUnused
|
|
}
|
|
|
|
func (s *stubLister) List(_ context.Context, _ string) ([]string, error) {
|
|
return nil, errStubUnused
|
|
}
|
|
|
|
func (s *stubLister) Info() storage.Info {
|
|
return storage.Info{}
|
|
}
|