Merge pull request #32 from shamaton/case_crash

Return error when strange data is received

.github/workflows/test.yml

@@ -7,33 +7,32 @@ env:
   WORKSPACE: ${{ github.workspace }}/src/github.com/${{ github.repository }}
 
 jobs:
-
   test:
     defaults:
       run:
         working-directory: ${{ env.WORKSPACE }}
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        go: [1.13, 1.14, 1.15, 1.16]
+        go: [1.17, 1.18, 1.19]
     name: ${{ matrix.os }} @ Go ${{ matrix.go }}
     runs-on: ${{ matrix.os }}
     steps:
       - name: Set up Go ${{ matrix.go }}
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v3
         with:
           go-version: ${{ matrix.go }}
 
       - name: Cache
-        uses: actions/cache@v2.1.0
+        uses: actions/cache@v3
         with:
           path: ~/go/pkg/mod
           key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
           restore-keys: |
             ${{ runner.os }}-go-
 
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           path: ${{ env.WORKSPACE }}
 
@@ -44,7 +43,7 @@ jobs:
         run: go test -v --coverpkg=github.com/shamaton/msgpack/... --coverprofile=coverage.coverprofile --covermode=atomic ./...
 
       - name: Upload coverage to Codecov
-        if: success() && matrix.go == 1.15 && matrix.os == 'ubuntu-latest'
+        if: success() && matrix.go == 1.19 && matrix.os == 'ubuntu-latest'
         uses: codecov/codecov-action@v1
         with:
           token:
@@ -56,11 +55,12 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           path: ${{ env.WORKSPACE }}
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: reviewdog/action-golangci-lint@v2
         with:
-          version: v1.29
-          working-directory: ${{ env.WORKSPACE }}
+          workdir: ${{ env.WORKSPACE }}
+          level: warning
+          reporter: github-pr-review

crash_test.go

@@ -0,0 +1,87 @@
+package msgpack_test
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"runtime"
+	"sync"
+	"testing"
+
+	"github.com/shamaton/msgpack/v2"
+)
+
+var crashDir = filepath.Join("testdata", "crashers")
+
+func TestCrashBinary(t *testing.T) {
+	entries, err := os.ReadDir(crashDir)
+	if err != nil {
+		t.Fatalf("os.ReadDir error. err: %+v", err)
+	}
+
+	ch := make(chan string, len(entries))
+
+	// worker
+	wg := sync.WaitGroup{}
+	for i := 0; i < runtime.NumCPU(); i++ {
+		wg.Add(1)
+		go check(t, &wg, ch)
+	}
+
+	for _, entry := range entries {
+		ch <- filepath.Join(crashDir, entry.Name())
+	}
+	close(ch)
+	wg.Wait()
+}
+
+func check(t *testing.T, wg *sync.WaitGroup, ch <-chan string) {
+	var (
+		path string
+		ok   bool
+		data []byte
+	)
+	t.Helper()
+	defer wg.Done()
+	defer func() {
+		if e := recover(); e != nil {
+			t.Logf("panic occurred.\nfile: %s\nlen: %d\nbin: % x\nerr: %+v",
+				path, len(data), data, e,
+			)
+		}
+	}()
+
+	for {
+		path, ok = <-ch // closeされると ok が false になる
+		if !ok {
+			return
+		}
+
+		file, err := os.Open(path)
+		if err != nil {
+			t.Logf("%s file open error. err: %+v", path, err)
+			t.Fail()
+			return
+		}
+
+		data, err = io.ReadAll(file)
+		if err != nil {
+			t.Logf("%s io.ReadAll error. err: %+v", path, err)
+			t.Fail()
+			return
+		}
+
+		var r interface{}
+		err = msgpack.Unmarshal(data, &r)
+		if err == nil {
+			t.Logf("err should be occurred.\nname: %s\nlen: %d\nbin: % x",
+				file.Name(), len(data), data,
+			)
+			t.Fail()
+			return
+		}
+
+		path = ""
+		data = nil
+	}
+}

internal/decoding/bin.go

@@ -17,20 +17,32 @@ func (d *decoder) isCodeBin(v byte) bool {
 }
 
 func (d *decoder) asBin(offset int, k reflect.Kind) ([]byte, int, error) {
-	code, offset := d.readSize1(offset)
+	code, offset, err := d.readSize1(offset)
+	if err != nil {
+		return emptyBytes, 0, err
+	}
 
 	switch code {
 	case def.Bin8:
-		l, offset := d.readSize1(offset)
+		l, offset, err := d.readSize1(offset)
+		if err != nil {
+			return emptyBytes, 0, err
+		}
 		o := offset + int(uint8(l))
 		return d.data[offset:o], o, nil
 	case def.Bin16:
-		bs, offset := d.readSize2(offset)
+		bs, offset, err := d.readSize2(offset)
 		o := offset + int(binary.BigEndian.Uint16(bs))
+		if err != nil {
+			return emptyBytes, 0, err
+		}
 		return d.data[offset:o], o, nil
 	case def.Bin32:
-		bs, offset := d.readSize4(offset)
+		bs, offset, err := d.readSize4(offset)
 		o := offset + int(binary.BigEndian.Uint32(bs))
+		if err != nil {
+			return emptyBytes, 0, err
+		}
 		return d.data[offset:o], o, nil
 	}
 

internal/decoding/complex.go

@@ -10,27 +10,48 @@ import (
 )
 
 func (d *decoder) asComplex64(offset int, k reflect.Kind) (complex64, int, error) {
-	code, offset := d.readSize1(offset)
+	code, offset, err := d.readSize1(offset)
+	if err != nil {
+		return complex(0, 0), 0, err
+	}
 
 	switch code {
 	case def.Fixext8:
-		t, offset := d.readSize1(offset)
+		t, offset, err := d.readSize1(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		if int8(t) != def.ComplexTypeCode() {
 			return complex(0, 0), 0, fmt.Errorf("fixext8. complex type is diffrent %d, %d", t, def.ComplexTypeCode())
 		}
-		rb, offset := d.readSize4(offset)
-		ib, offset := d.readSize4(offset)
+		rb, offset, err := d.readSize4(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
+		ib, offset, err := d.readSize4(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		r := math.Float32frombits(binary.BigEndian.Uint32(rb))
 		i := math.Float32frombits(binary.BigEndian.Uint32(ib))
 		return complex(r, i), offset, nil
 
 	case def.Fixext16:
-		t, offset := d.readSize1(offset)
+		t, offset, err := d.readSize1(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		if int8(t) != def.ComplexTypeCode() {
 			return complex(0, 0), 0, fmt.Errorf("fixext16. complex type is diffrent %d, %d", t, def.ComplexTypeCode())
 		}
-		rb, offset := d.readSize8(offset)
-		ib, offset := d.readSize8(offset)
+		rb, offset, err := d.readSize8(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
+		ib, offset, err := d.readSize8(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		r := math.Float64frombits(binary.BigEndian.Uint64(rb))
 		i := math.Float64frombits(binary.BigEndian.Uint64(ib))
 		return complex64(complex(r, i)), offset, nil
@@ -41,27 +62,48 @@ func (d *decoder) asComplex64(offset int, k reflect.Kind) (complex64, int, error
 }
 
 func (d *decoder) asComplex128(offset int, k reflect.Kind) (complex128, int, error) {
-	code, offset := d.readSize1(offset)
+	code, offset, err := d.readSize1(offset)
+	if err != nil {
+		return complex(0, 0), 0, err
+	}
 
 	switch code {
 	case def.Fixext8:
-		t, offset := d.readSize1(offset)
+		t, offset, err := d.readSize1(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		if int8(t) != def.ComplexTypeCode() {
 			return complex(0, 0), 0, fmt.Errorf("fixext8. complex type is diffrent %d, %d", t, def.ComplexTypeCode())
 		}
-		rb, offset := d.readSize4(offset)
-		ib, offset := d.readSize4(offset)
+		rb, offset, err := d.readSize4(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
+		ib, offset, err := d.readSize4(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		r := math.Float32frombits(binary.BigEndian.Uint32(rb))
 		i := math.Float32frombits(binary.BigEndian.Uint32(ib))
 		return complex128(complex(r, i)), offset, nil
 
 	case def.Fixext16:
-		t, offset := d.readSize1(offset)
+		t, offset, err := d.readSize1(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		if int8(t) != def.ComplexTypeCode() {
 			return complex(0, 0), 0, fmt.Errorf("fixext16. complex type is diffrent %d, %d", t, def.ComplexTypeCode())
 		}
-		rb, offset := d.readSize8(offset)
-		ib, offset := d.readSize8(offset)
+		rb, offset, err := d.readSize8(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
+		ib, offset, err := d.readSize8(offset)
+		if err != nil {
+			return complex(0, 0), 0, err
+		}
 		r := math.Float64frombits(binary.BigEndian.Uint64(rb))
 		i := math.Float64frombits(binary.BigEndian.Uint64(ib))
 		return complex(r, i), offset, nil

internal/decoding/decoding.go

@@ -18,10 +18,9 @@ type decoder struct {
 func Decode(data []byte, v interface{}, asArray bool) error {
 	d := decoder{data: data, asArray: asArray}
 
-	if d.data == nil {
-		return fmt.Errorf("data is nil")
+	if d.data == nil || len(d.data) < 1 {
+		return fmt.Errorf("data is empty")
 	}
-
 	rv := reflect.ValueOf(v)
 	if rv.Kind() != reflect.Ptr {
 		return fmt.Errorf("holder must set pointer value. but got: %t", v)
@@ -120,7 +119,10 @@ func (d *decoder) decode(rv reflect.Value, offset int) (int, error) {
 			if err != nil {
 				return 0, err
 			}
-			bs, offset := d.asStringByteByLength(offset, l, k)
+			bs, offset, err := d.asStringByteByLength(offset, l, k)
+			if err != nil {
+				return 0, err
+			}
 			rv.SetBytes(bs)
 			return offset, nil
 		}
@@ -131,6 +133,10 @@ func (d *decoder) decode(rv reflect.Value, offset int) (int, error) {
 			return 0, err
 		}
 
+		if err = d.hasRequiredLeastSliceSize(o, l); err != nil {
+			return 0, err
+		}
+
 		// check fixed type
 		fixedOffset, found, err := d.asFixedSlice(rv, o, l)
 		if err != nil {
@@ -201,7 +207,10 @@ func (d *decoder) decode(rv reflect.Value, offset int) (int, error) {
 			if l > rv.Len() {
 				return 0, fmt.Errorf("%v len is %d, but msgpack has %d elements", rv.Type(), rv.Len(), l)
 			}
-			bs, offset := d.asStringByteByLength(offset, l, k)
+			bs, offset, err := d.asStringByteByLength(offset, l, k)
+			if err != nil {
+				return 0, err
+			}
 			for i, b := range bs {
 				rv.Index(i).SetUint(uint64(b))
 			}
@@ -218,6 +227,10 @@ func (d *decoder) decode(rv reflect.Value, offset int) (int, error) {
 			return 0, fmt.Errorf("%v len is %d, but msgpack has %d elements", rv.Type(), rv.Len(), l)
 		}
 
+		if err = d.hasRequiredLeastSliceSize(o, l); err != nil {
+			return 0, err
+		}
+
 		// create array dynamically
 		for i := 0; i < l; i++ {
 			o, err = d.decode(rv.Index(i), o)
@@ -240,6 +253,10 @@ func (d *decoder) decode(rv reflect.Value, offset int) (int, error) {
 			return 0, err
 		}
 
+		if err = d.hasRequiredLeastMapSize(o, l); err != nil {
+			return 0, err
+		}
+
 		// check fixed type
 		fixedOffset, found, err := d.asFixedMap(rv, o, l)
 		if err != nil {