Merge pull request #441 from n3integration/bug/snmp-unmarshal-extra-b…

…ounds-check

unmarshal: fix panic from reading beyond slice

marshal.go

@@ -1022,7 +1022,7 @@ func (x *GoSNMP) unmarshalHeader(packet []byte, response *SnmpPacket) (int, erro
 	}
 
 	cursor += count
-	if cursor > len(packet) {
+	if cursor >= len(packet) {
 		return 0, fmt.Errorf("error parsing SNMP packet, packet length %d cursor %d", len(packet), cursor)
 	}
 

marshal_test.go

@@ -367,6 +367,14 @@ func TestEnmarshalMsg(t *testing.T) {
 
 // -- Unmarshal -----------------------------------------------------------------
 
+var testsUnmarshalErr = []struct {
+	in func() []byte
+}{
+	{
+		panicUnmarshalHeader,
+	},
+}
+
 var testsUnmarshal = []struct {
 	in  func() []byte
 	out *SnmpPacket
@@ -731,6 +739,25 @@ var testsUnmarshal = []struct {
 	},
 }
 
+func TestUnmarshalErrors(t *testing.T) {
+	Default.Logger = NewLogger(log.New(io.Discard, "", 0))
+
+	for i, test := range testsUnmarshalErr {
+		funcName := runtime.FuncForPC(reflect.ValueOf(test.in).Pointer()).Name()
+		splitedFuncName := strings.Split(funcName, ".")
+		funcName = splitedFuncName[len(splitedFuncName)-1]
+		t.Run(fmt.Sprintf("%v-%v", i, funcName), func(t *testing.T) {
+			vhandle := GoSNMP{}
+			vhandle.Logger = Default.Logger
+			testBytes := test.in()
+			_, err := vhandle.SnmpDecodePacket(testBytes)
+			if err == nil {
+				t.Errorf("#%s: SnmpDecodePacket() err expected, but not returned", funcName)
+			}
+		})
+	}
+}
+
 func TestUnmarshal(t *testing.T) {
 	Default.Logger = NewLogger(log.New(io.Discard, "", 0))
 
@@ -842,6 +869,14 @@ func TestUnmarshal(t *testing.T) {
 * Frame, Ethernet II, IP and UDP layers removed from generated bytes
 */
 
+/*
+panicUnmarshalHeader tests a boundary condition that results in a panic
+when unmarshalling the SNMP header (see also https://github.com/gosnmp/gosnmp/issues/440)
+*/
+func panicUnmarshalHeader() []byte {
+	return []byte("0\x04\x02\x020\x03")
+}
+
 /*
 kyoceraResponseBytes corresponds to the response section of this snmpget