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

Fix potential panic due to slice out of index issue, it may happen if… #403

Closed
wants to merge 1 commit into from

Conversation

efancier-cn
Copy link

… snmpv3 priv password not match, return value of parseLength may be negtive

… snmpv3 priv password not match, return value of parseLength may be negtive

Signed-off-by: roli <robin_li@126.com>
Signed-off-by: Robin(YuanBing) Li <robin_li@126.com>
@SuperQ
Copy link
Contributor

SuperQ commented May 13, 2022

parseLength() already has a check for this.

But, maybe we should fix the output of that function to be a uint to make it more clear.

@efancier-cn
Copy link
Author

change the output of that function parseLength() to be uint will ideal - I am just trying to avoid break other functions since it is widely used. the issue itself is real and can be reliably replicated.

@SuperQ
Copy link
Contributor

SuperQ commented May 14, 2022

I don't see how parseLength can return a negative value.

It has an explicit check and returns 0

@SuperQ
Copy link
Contributor

SuperQ commented May 14, 2022

Maybe the real issue is the cursor is negative?

@TomSellers
Copy link
Contributor

TomSellers commented Aug 29, 2023

This came up while fuzzing in PR #443 and a fix for it was add there. length can overflow during the loop and become negative. Later additions to length in the loop can make it positive but it is still less than cursor. When the calling code uses these two values to index into a slice a panic can occur.

panic: runtime error: slice bounds out of range [109:108]

#443 (comment)

Reproducer (panicUnmarshalParseRawFieldTimeTicks in the above PR)

package main

import "github.com/gosnmp/gosnmp"

func main() {
	payload := []byte{
		0x30, 0x81, 0xc5, 0x43, 0x01, 0x30, 0x43, 0x06, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0xa2, 0x81,
		0xb7, 0x43, 0x04, 0x30, 0x30, 0x30, 0x30, 0x43, 0x01, 0x30, 0x43, 0xeb, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0xff,
		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
		0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30,
	}
	g := &gosnmp.GoSNMP{}
	_, _ = g.SnmpDecodePacket(payload)
}

Output

go run examples/example-r0/main.go
panic: runtime error: slice bounds out of range [109:108]

goroutine 1 [running]:
github.com/gosnmp/gosnmp.parseRawField({{0x0, 0x0}}, {0x1400013401a, 0xae, 0xae}, {0x104ad8006, 0xb})
	/Users/username/git/gosnmp/helper.go:724 +0xa34
github.com/gosnmp/gosnmp.(*GoSNMP).unmarshalResponse(0x14000054de0, {0x1400013400e, 0xba, 0xba}, 0x14000136000)
	/Users/username/git/gosnmp/marshal.go:1167 +0x648
github.com/gosnmp/gosnmp.(*GoSNMP).unmarshalPayload(0x14000054de0, {0x14000134000, 0xc8, 0xc8}, 0xe, 0x14000136000)
	/Users/username/git/gosnmp/marshal.go:1078 +0x164
github.com/gosnmp/gosnmp.(*GoSNMP).SnmpDecodePacket(0x14000054de0, {0x14000134000, 0xc8, 0xc8})
	/Users/username/git/gosnmp/gosnmp.go:554 +0x174
main.main()
	/Users/username/git/gosnmp/examples/example-r0/main.go:26 +0x118
exit status 2

@SuperQ
Copy link
Contributor

SuperQ commented Sep 5, 2023

Fixed in #443.

@SuperQ SuperQ closed this Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants