Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unmarshal: fix panic from reading beyond slice #441

Merged
merged 3 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
35 changes: 35 additions & 0 deletions marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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

Expand Down