Skip to content

Commit

Permalink
Fix race in LFS ContentStore.Put(...) (go-gitea#14895)
Browse files Browse the repository at this point in the history
Backport go-gitea#14895

Continuing on from go-gitea#14888

The previous implementation has race whereby an incomplete upload or
hash mismatch upload can end up in the ContentStore. This PR moves the
validation into the reader so that if there is a hash error or size
mismatch the reader will return with an error instead of an io.EOF
causing the storage to abort the storage.

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath committed Mar 6, 2021
1 parent 8e79298 commit 0e2396e
Showing 1 changed file with 51 additions and 11 deletions.
62 changes: 51 additions & 11 deletions modules/lfs/content_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/hex"
"errors"
"fmt"
"hash"
"io"
"os"

Expand Down Expand Up @@ -66,30 +67,27 @@ func (s *ContentStore) Get(meta *models.LFSMetaObject, fromByte int64) (io.ReadC

// Put takes a Meta object and an io.Reader and writes the content to the store.
func (s *ContentStore) Put(meta *models.LFSMetaObject, r io.Reader) error {
hash := sha256.New()
rd := io.TeeReader(r, hash)
p := meta.RelativePath()
written, err := s.Save(p, rd)

// Wrap the provided reader with an inline hashing and size checker
wrappedRd := newHashingReader(meta.Size, meta.Oid, r)

// now pass the wrapped reader to Save - if there is a size mismatch or hash mismatch then
// the errors returned by the newHashingReader should percolate up to here
written, err := s.Save(p, wrappedRd)
if err != nil {
log.Error("Whilst putting LFS OID[%s]: Failed to copy to tmpPath: %s Error: %v", meta.Oid, p, err)
return err
}

// This shouldn't happen but it is sensible to test
if written != meta.Size {
if err := s.Delete(p); err != nil {
log.Error("Cleaning the LFS OID[%s] failed: %v", meta.Oid, err)
}
return errSizeMismatch
}

shaStr := hex.EncodeToString(hash.Sum(nil))
if shaStr != meta.Oid {
if err := s.Delete(p); err != nil {
log.Error("Cleaning the LFS OID[%s] failed: %v", meta.Oid, err)
}
return errHashMismatch
}

return nil
}

Expand Down Expand Up @@ -118,3 +116,45 @@ func (s *ContentStore) Verify(meta *models.LFSMetaObject) (bool, error) {

return true, nil
}

type hashingReader struct {
internal io.Reader
currentSize int64
expectedSize int64
hash hash.Hash
expectedHash string
}

func (r *hashingReader) Read(b []byte) (int, error) {
n, err := r.internal.Read(b)

if n > 0 {
r.currentSize += int64(n)
wn, werr := r.hash.Write(b[:n])
if wn != n || werr != nil {
return n, werr
}
}

if err != nil && err == io.EOF {
if r.currentSize != r.expectedSize {
return n, errSizeMismatch
}

shaStr := hex.EncodeToString(r.hash.Sum(nil))
if shaStr != r.expectedHash {
return n, errHashMismatch
}
}

return n, err
}

func newHashingReader(expectedSize int64, expectedHash string, reader io.Reader) *hashingReader {
return &hashingReader{
internal: reader,
expectedSize: expectedSize,
expectedHash: expectedHash,
hash: sha256.New(),
}
}

0 comments on commit 0e2396e

Please sign in to comment.