Fix panic on unpack nil or smaller input size (#161)

* fix decode with nil or zero/wrong length data * check input length to avoid panic because of slice bounds out of range
moov-io · Feb 9, 2022 · 07d52aa · 07d52aa
1 parent f72989c
commit 07d52aa
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 4 deletions.
diff --git a/encoding/ascii.go b/encoding/ascii.go
@@ -1,6 +1,8 @@
 package encoding
 
-import "fmt"
+import (
+	"fmt"
+)
 
 var ASCII Encoder = &asciiEncoder{}
 
@@ -20,6 +22,9 @@ func (e asciiEncoder) Encode(data []byte) ([]byte, error) {
 
 func (e asciiEncoder) Decode(data []byte, length int) ([]byte, int, error) {
 	// read only 'length' bytes (1 byte - 1 ASCII character)
+	if len(data) < length {
+		return nil, 0, fmt.Errorf("not enough data to decode. expected len %d, got %d", length, len(data))
+	}
 	data = data[:length]
 	var out []byte
 	for _, r := range data {

diff --git a/encoding/ascii_test.go b/encoding/ascii_test.go
@@ -18,6 +18,14 @@ func TestASCII(t *testing.T) {
 
 		_, _, err = enc.Decode([]byte("hello, 世界!"), 10)
 		require.Error(t, err)
+
+		_, _, err = enc.Decode([]byte("hello"), 6)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 6, got 5")
+
+		_, _, err = enc.Decode(nil, 6)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 6, got 0")
 	})
 
 	t.Run("Encode", func(t *testing.T) {

diff --git a/encoding/bcd.go b/encoding/bcd.go
@@ -1,6 +1,8 @@
 package encoding
 
 import (
+	"fmt"
+
 	"github.com/yerden/go-util/bcd"
 )
 
@@ -33,6 +35,10 @@ func (e *bcdEncoder) Decode(src []byte, length int) ([]byte, int, error) {
 	// how many bytes we will read
 	read := bcd.EncodedLen(decodedLen)
 
+	if len(src) < read {
+		return nil, 0, fmt.Errorf("not enough data to decode. expected len %d, got %d", read, len(src))
+	}
+
 	dec := bcd.NewDecoder(bcd.Standard)
 	dst := make([]byte, decodedLen)
 	_, err := dec.Decode(dst, src[:read])

diff --git a/encoding/bcd_test.go b/encoding/bcd_test.go
@@ -37,6 +37,14 @@ func TestBCD(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, []byte("2143"), res)
 		require.Equal(t, 2, read)
+
+		_, _, err = BCD.Decode([]byte{0x21, 0x43}, 6)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 3, got 2")
+
+		_, _, err = BCD.Decode(nil, 6)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 3, got 0")
 	})
 
 	t.Run("Encode", func(t *testing.T) {

diff --git a/encoding/ebcdic.go b/encoding/ebcdic.go
@@ -1,5 +1,7 @@
 package encoding
 
+import "fmt"
+
 //Encoding map taken from
 //http://www.ibm.com/support/knowledgecenter/SSZJPZ_11.3.0/com.ibm.swg.im.iis.ds.parjob.adref.doc/topics/r_deeadvrf_EBCDIC_to_ASCII.html
 
@@ -86,6 +88,9 @@ func (e *ebcdicEncoder) Encode(src []byte) ([]byte, error) {
 }
 
 func (e *ebcdicEncoder) Decode(src []byte, length int) ([]byte, int, error) {
+	if len(src) < length {
+		return nil, 0, fmt.Errorf("not enough data to decode. expected len %d, got %d", length, len(src))
+	}
 	src = src[:length]
 	var dst []byte
 	for _, v := range src {

diff --git a/encoding/ebcdic_test.go b/encoding/ebcdic_test.go
@@ -14,6 +14,14 @@ func TestEBCDIC(t *testing.T) {
 		require.Equal(t, []byte{0x12, 0x94}, res)
 		require.Equal(t, 2, read)
 
+		_, _, err = EBCDIC.Decode([]byte{0x12, 0x34}, 3)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 3, got 2")
+
+		_, _, err = EBCDIC.Decode(nil, 6)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 6, got 0")
+
 	})
 
 	t.Run("Encode", func(t *testing.T) {

diff --git a/encoding/hex_test.go b/encoding/hex_test.go
@@ -14,6 +14,10 @@ func TestHexToASCIIEncoder(t *testing.T) {
 	require.Equal(t, 6, read)
 	require.Equal(t, []byte{0xAA, 0xBB, 0xCC}, got)
 
+	_, _, err = enc.Decode(nil, 3)
+	require.Error(t, err)
+	require.EqualError(t, err, "not enough data to read")
+
 	got, err = enc.Encode([]byte{0xAA, 0xBB, 0xCC})
 	require.NoError(t, err)
 	require.Equal(t, []byte("AABBCC"), got)

diff --git a/encoding/lbcd.go b/encoding/lbcd.go
@@ -1,6 +1,8 @@
 package encoding
 
 import (
+	"fmt"
+
 	"github.com/yerden/go-util/bcd"
 )
 
@@ -33,6 +35,11 @@ func (e *lBCDEncoder) Decode(src []byte, length int) ([]byte, int, error) {
 
 	dec := bcd.NewDecoder(bcd.Standard)
 	dst := make([]byte, decodedLen)
+
+	if len(src) < read {
+		return nil, 0, fmt.Errorf("not enough data to decode. expected len %d, got %d", read, len(src))
+	}
+
 	_, err := dec.Decode(dst, src)
 	if err != nil {
 		return nil, 0, err

diff --git a/encoding/lbcd_test.go b/encoding/lbcd_test.go
@@ -19,6 +19,14 @@ func TestLBCD(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, []byte("123"), res)
 		require.Equal(t, 2, read)
+
+		_, _, err = LBCD.Decode([]byte{0x12, 0x30}, 5)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 3, got 2")
+
+		_, _, err = LBCD.Decode(nil, 5)
+		require.Error(t, err)
+		require.EqualError(t, err, "not enough data to decode. expected len 3, got 0")
 	})
 
 	t.Run("Encode", func(t *testing.T) {

diff --git a/field/composite_test.go b/field/composite_test.go
@@ -429,7 +429,7 @@ func TestCompositePacking(t *testing.T) {
 		read, err := composite.Unpack([]byte("ABCD123"))
 		require.Equal(t, 0, read)
 		require.Error(t, err)
-		require.EqualError(t, err, "data length: 4 does not match aggregate data read from decoded subfields: 7")
+		require.EqualError(t, err, "failed to unpack subfield 3: failed to decode content: not enough data to decode. expected len 3, got 0")
 	})
 
 	t.Run("Unpack correctly deserialises bytes to the data struct", func(t *testing.T) {

diff --git a/message_test.go b/message_test.go
@@ -531,6 +531,24 @@ func TestPackUnpack(t *testing.T) {
 		assert.Equal(t, "000000000501", data.F55.F9F02.Value)
 		assert.Equal(t, "Another test text", data.F120.Value)
 	})
+
+	t.Run("Unpack nil", func(t *testing.T) {
+		message := NewMessage(spec)
+
+		err := message.Unpack(nil)
+
+		require.Error(t, err)
+	})
+
+	t.Run("Unpack short mti", func(t *testing.T) {
+		message := NewMessage(spec)
+
+		rawMsg := []byte{0x30, 0x31}
+
+		err := message.Unpack([]byte(rawMsg))
+
+		require.Error(t, err)
+	})
 }
 
 func TestMessageJSON(t *testing.T) {

diff --git a/prefix/ascii.go b/prefix/ascii.go
@@ -34,7 +34,7 @@ func (p *asciiVarPrefixer) EncodeLength(maxLen, dataLen int) ([]byte, error) {
 
 func (p *asciiVarPrefixer) DecodeLength(maxLen int, data []byte) (int, int, error) {
 	if len(data) < p.Digits {
-		return 0, 0, fmt.Errorf("not enought data length: %d to read: %d byte digits", len(data), p.Digits)
+		return 0, 0, fmt.Errorf("not enough data length: %d to read: %d byte digits", len(data), p.Digits)
 	}
 
 	dataLen, err := strconv.Atoi(string(data[:p.Digits]))

diff --git a/prefix/ascii_test.go b/prefix/ascii_test.go
@@ -33,7 +33,7 @@ func TestAsciiVarPrefixer_DecodeLengthMaxLengthValidation(t *testing.T) {
 
 	_, _, err := pref.DecodeLength(20, []byte("22"))
 
-	require.Contains(t, err.Error(), "not enought data length: 2 to read: 3 byte digits")
+	require.Contains(t, err.Error(), "not enough data length: 2 to read: 3 byte digits")
 }
 
 func TestAsciiVarPrefixer_LHelpers(t *testing.T) {