do not crash readXLMetaNoData - if the `xl.meta` has incorrect content (#14538)

```
tmp = buf[want:]
```

Would potentially crash when `buf` is truncated for some reason
and does not have the expected bytes, this is of course considered
not normal and is an odd situation. But we do not need to crash
here instead allow for errors to be returned and let callers handle
the errors.
This commit is contained in:
Harshavardhana 2022-03-14 09:07:46 -07:00 committed by GitHub
parent 6187440f35
commit cf94d1f1f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 38 additions and 5 deletions

BIN
cmd/testdata/xl.meta-corrupt.gz vendored Normal file

Binary file not shown.

View File

@ -658,7 +658,7 @@ func readXLMetaNoData(r io.Reader, size int64) ([]byte, error) {
buf := metaDataPoolGet()[:initial]
_, err := io.ReadFull(r, buf)
if err != nil {
return nil, fmt.Errorf("readXLMetaNoData.ReadFull: %w", err)
return nil, fmt.Errorf("readXLMetaNoData(io.ReadFull): %w", err)
}
readMore := func(n int64) error {
has := int64(len(buf))
@ -679,9 +679,9 @@ func readXLMetaNoData(r io.Reader, size int64) ([]byte, error) {
if err != nil {
if errors.Is(err, io.EOF) {
// Returned if we read nothing.
return fmt.Errorf("readXLMetaNoData.readMore: %w", io.ErrUnexpectedEOF)
err = io.ErrUnexpectedEOF
}
return fmt.Errorf("readXLMetaNoData.readMore: %w", err)
return fmt.Errorf("readXLMetaNoData(readMore): %w", err)
}
return nil
}
@ -699,7 +699,7 @@ func readXLMetaNoData(r io.Reader, size int64) ([]byte, error) {
case 1, 2, 3:
sz, tmp, err := msgp.ReadBytesHeader(tmp)
if err != nil {
return nil, err
return nil, fmt.Errorf("readXLMetaNoData(read_meta): uknown metadata version %w", err)
}
want := int64(sz) + int64(len(buf)-len(tmp))
@ -720,10 +720,14 @@ func readXLMetaNoData(r io.Reader, size int64) ([]byte, error) {
return nil, err
}
if int64(len(buf)) < want {
return nil, fmt.Errorf("buffer shorter than expected (buflen: %d, want: %d): %w", len(buf), want, errFileCorrupt)
}
tmp = buf[want:]
_, after, err := msgp.ReadUint32Bytes(tmp)
if err != nil {
return nil, err
return nil, fmt.Errorf("readXLMetaNoData(read_meta): unknown metadata version %w", err)
}
want += int64(len(tmp) - len(after))

View File

@ -18,10 +18,13 @@
package cmd
import (
"bufio"
"bytes"
"compress/gzip"
"encoding/json"
"fmt"
"io"
"os"
"sort"
"testing"
"time"
@ -34,6 +37,29 @@ import (
"github.com/minio/minio/internal/ioutil"
)
func TestReadXLMetaNoData(t *testing.T) {
f, err := os.Open("testdata/xl.meta-corrupt.gz")
if err != nil {
t.Fatal(err)
}
defer f.Close()
gz, err := gzip.NewReader(bufio.NewReader(f))
if err != nil {
t.Fatal(err)
}
buf, err := io.ReadAll(gz)
if err != nil {
t.Fatal(err)
}
_, err = readXLMetaNoData(bytes.NewReader(buf), int64(len(buf)))
if err == nil {
t.Fatal("expected error but returned success")
}
}
func TestXLV2FormatData(t *testing.T) {
failOnErr := func(err error) {
t.Helper()

View File

@ -398,6 +398,9 @@ func (s *xlStorage) readMetadataWithDMTime(ctx context.Context, itemPath string)
}
}
buf, err := readXLMetaNoData(f, stat.Size())
if err != nil {
return nil, stat.ModTime().UTC(), fmt.Errorf("%w -> %s", err, itemPath)
}
return buf, stat.ModTime().UTC(), err
}