diff --git a/common/geodata/geodat_loader.go b/common/geodata/geodat_loader.go index 2c8fc8627..658b7481d 100644 --- a/common/geodata/geodat_loader.go +++ b/common/geodata/geodat_loader.go @@ -17,12 +17,12 @@ import ( ) func checkFile(file, code string) error { - r, err := filesystem.OpenAsset(file) + r, size, err := filesystem.OpenAsset(file) if err != nil { return errors.New("failed to open ", file).Base(err) } defer r.Close() - if _, err := find(r, []byte(code), false); err != nil { + if _, err := find(r, []byte(code), false, size); err != nil { return errors.New("failed to check code ", code, " from ", file).Base(err) } return nil @@ -30,12 +30,12 @@ func checkFile(file, code string) error { func loadFile(file, code string) ([]byte, error) { runtime.GC() // peak mem - r, err := filesystem.OpenAsset(file) + r, size, err := filesystem.OpenAsset(file) if err != nil { return nil, errors.New("failed to open ", file).Base(err) } defer r.Close() - bs, err := find(r, []byte(code), true) + bs, err := find(r, []byte(code), true, size) if err != nil { return nil, errors.New("failed to load code ", code, " from ", file).Base(err) } @@ -60,13 +60,13 @@ func loadIP(file, code string) ([]*CIDR, error) { // unmarshalling it into a []*Domain, so value is only valid during fn. func loadSite(file, code, attrs string, fn func(Domain_Type, []byte)) error { runtime.GC() // peak mem - r, err := filesystem.OpenAsset(file) + r, size, err := filesystem.OpenAsset(file) if err != nil { return errors.New("failed to open ", file).Base(err) } defer r.Close() br := bufio.NewReaderSize(r, 64*1024) - n, err := seek(br, []byte(code)) + n, err := seek(br, []byte(code), size) if err != nil { return errors.New("failed to load code ", code, " from ", file).Base(err) } @@ -126,9 +126,9 @@ func decodeVarint(br *bufio.Reader) (uint64, error) { return 0, errors.New("varint overflow") } -func find(r io.Reader, code []byte, readBody bool) ([]byte, error) { +func find(r io.Reader, code []byte, readBody bool, size int64) ([]byte, error) { br := bufio.NewReaderSize(r, 64*1024) - bodyL, err := seek(br, code) + bodyL, err := seek(br, code, size) if err != nil || !readBody { return nil, err } @@ -140,7 +140,7 @@ func find(r io.Reader, code []byte, readBody bool) ([]byte, error) { } // seek advances br to the body of the entry for code and returns the body length. -func seek(br *bufio.Reader, code []byte) (int, error) { +func seek(br *bufio.Reader, code []byte, size int64) (int, error) { codeL := len(code) if codeL == 0 { return 0, errors.New("empty code") @@ -156,10 +156,10 @@ func seek(br *bufio.Reader, code []byte) (int, error) { if err != nil { return 0, err } - bodyL := int(x) - if bodyL <= 0 { - return 0, errors.New("invalid body length: ", bodyL) + if x == 0 || x > uint64(size) || x > uint64(^uint(0)>>1) { + return 0, errors.New("invalid body length: ", x) } + bodyL := int(x) // Peek no more than the buffer holds: a code longer than the buffer cannot match a single // length byte anyway, so a short peek only skips it, as base find (io.ReadFull) does. diff --git a/common/geodata/geodat_loader_test.go b/common/geodata/geodat_loader_test.go index 42db3b7d1..10281ae79 100644 --- a/common/geodata/geodat_loader_test.go +++ b/common/geodata/geodat_loader_test.go @@ -28,6 +28,29 @@ func TestLoadFileRejectsVarintOverflow(t *testing.T) { } } +func TestLoadFileRejectsOversizedEntry(t *testing.T) { + payload := []byte{ + 0x0a, 0x03, 0x0a, 0x01, 'A', // A valid entry before the oversized one. + 0x0a, 0x80, 0x80, 0x80, 0x80, 0x04, // Entry length: 1 GiB. + 0x0a, 0x04, 'T', 'E', 'S', 'T', + } + dir := t.TempDir() + t.Setenv("xray.location.asset", dir) + if err := os.WriteFile(filepath.Join(dir, "oversized.dat"), payload, 0o600); err != nil { + t.Fatal(err) + } + // Check without allocating the body first, so a regression fails safely. + if err := checkFile("oversized.dat", "TEST"); err == nil { + t.Fatal("accepted an entry length larger than the file") + } + if _, err := loadFile("oversized.dat", "TEST"); err == nil || !strings.Contains(err.Error(), "invalid body length") { + t.Fatalf("want invalid body length, got %v", err) + } + if err := loadSite("oversized.dat", "TEST", "", func(Domain_Type, []byte) {}); err == nil || !strings.Contains(err.Error(), "invalid body length") { + t.Fatalf("want invalid body length, got %v", err) + } +} + type siteEntry struct { Type Domain_Type Value string diff --git a/common/platform/filesystem/file.go b/common/platform/filesystem/file.go index d9d1d39fe..24cf112e2 100644 --- a/common/platform/filesystem/file.go +++ b/common/platform/filesystem/file.go @@ -34,12 +34,13 @@ func ReadAsset(file string) ([]byte, error) { return ReadFile(path) } -func OpenAsset(file string) (io.ReadCloser, error) { - path, _, err := getAssetFileLocation(file) +func OpenAsset(file string) (io.ReadCloser, int64, error) { + path, info, err := getAssetFileLocation(file) if err != nil { - return nil, err + return nil, 0, err } - return NewFileReader(path) + r, err := NewFileReader(path) + return r, info.Size(), err } func StatAsset(file string) (os.FileInfo, error) {