carverauto/serviceradar

Fork 0

SNMP OctetString/ObjectDescription conversion asserts byte instead of []byte, causing panic #692

New issue

Closed

opened 2026-03-28 04:27:30 +00:00 by mfreeman451 · 1 comment

mfreeman451 commented

2026-03-28 04:27:30 +00:00

Owner

Imported from GitHub.

Original GitHub issue: #2154
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/issues/2154
Original created: 2025-12-16T05:19:17Z

Summary

Context: The SNMP client in pkg/checker/snmp/client.go converts SNMP PDU variables returned from the gosnmp library into Go types for use in the monitoring system.
Bug: The convertOctetString and convertObjectDescription functions incorrectly attempt to type-assert variable.Value as a single byte instead of a byte slice []byte.
Actual vs. expected: The gosnmp library returns []byte for OctetString and ObjectDescription types, but the code tries to convert to a single byte, causing a panic.
Impact: Any SNMP query that retrieves OctetString or ObjectDescription values (including the most common OID sysDescr used for device identification) will crash the application with a panic.

Code with bug

func convertObjectDescription(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.(byte)) // <-- BUG 🔴 Should be []byte, not byte
}

func convertOctetString(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.(byte)) // <-- BUG 🔴 Should be []byte, not byte
}

Evidence

Example

The SNMP OID 1.3.6.1.2.1.1.1.0 (sysDescr - system description) is one of the most fundamental SNMP OIDs used to identify network devices. This OID returns a DisplayString type, which is encoded as an OctetString in SNMP.

When the gosnmp library queries this OID, it returns:

gosnmp.SnmpPDU{
    Name:  ".1.3.6.1.2.1.1.1.0",
    Type:  gosnmp.OctetString,
    Value: []byte("Linux foo.example.lan 2.6.32-573.1.1.v6.i686..."), // This is []byte
}

The code attempts:

string(variable.Value.(byte))

This type assertion fails because:

Actual type: []byte (also known as []uint8)
Expected by code: byte (also known as uint8)

The result is a panic: interface conversion: interface {} is []uint8, not uint8

Inconsistency with API documentation

Reference API documentation

From the gosnmp documentation at https://pkg.go.dev/github.com/gosnmp/gosnmp:

the Value of each variable returned by Get() implements interface{}. You could do a type switch...

Example from the documentation:

for _, variable := range result.Variables {
    switch variable.Type {
    case gosnmp.OctetString:
        bytes := variable.Value.([]byte)
        fmt.Printf("string: %s\n", string(bytes))
    }
}

The documentation clearly shows that OctetString values should be type-asserted to []byte, not byte.

Current API usage

pkg/checker/snmp/client.go lines 268 and 296:

func convertObjectDescription(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.(byte))
}

func convertOctetString(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.(byte))
}

Contradiction

The code attempts to convert variable.Value to a single byte, but the gosnmp library's API documentation and implementation clearly shows that OctetString and ObjectDescription values are returned as []byte (byte slices), not single bytes.

Inconsistency within the codebase

Reference code

pkg/checker/snmp/collector.go lines 376-386:

// convertString converts a string value to a string.
func (*SNMPCollector) convertString(value interface{}) (string, error) {
	switch v := value.(type) {
	case []byte:
		return string(v), nil
	case string:
		return v, nil
	default:
		return "", fmt.Errorf("%w %T", ErrInvalidStringType, value)
	}
}

Current code

pkg/checker/snmp/client.go lines 295-297:

func convertOctetString(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.(byte))
}

Comparison

The collector correctly handles string conversion by accepting []byte and converting it to string. The convertString function in the collector demonstrates the correct understanding that SNMP string values come as byte slices. However, the convertOctetString function in the client contradicts this by attempting to convert a single byte, which is inconsistent with both the gosnmp API and the codebase's own handling of similar data types.

Failing test

Test script

package snmp

import (
	"testing"

	"github.com/gosnmp/gosnmp"
)

// TestConvertOctetStringTypeBug tests the bug where convertOctetString
// incorrectly tries to convert Value to a single byte instead of []byte
func TestConvertOctetStringTypeBug(t *testing.T) {
	client := &SNMPClientImpl{}

	// This is what gosnmp actually returns for OctetString types
	variable := gosnmp.SnmpPDU{
		Name:  ".1.3.6.1.2.1.1.1.0",
		Type:  gosnmp.OctetString,
		Value: []byte("Test SNMP String"),
	}

	// This should work but will panic due to the bug
	_, err := client.convertVariable(variable)
	if err != nil {
		t.Fatalf("convertVariable failed: %v", err)
	}
}

// TestConvertObjectDescriptionTypeBug tests the bug where convertObjectDescription
// incorrectly tries to convert Value to a single byte instead of []byte
func TestConvertObjectDescriptionTypeBug(t *testing.T) {
	client := &SNMPClientImpl{}

	// ObjectDescription should also be []byte
	variable := gosnmp.SnmpPDU{
		Name:  ".1.3.6.1.2.1.1.2.0",
		Type:  gosnmp.ObjectDescription,
		Value: []byte("Object Description"),
	}

	// This should work but will panic due to the bug
	_, err := client.convertVariable(variable)
	if err != nil {
		t.Fatalf("convertVariable failed: %v", err)
	}
}

Test output

=== RUN   TestConvertOctetStringTypeBug
--- FAIL: TestConvertOctetStringTypeBug (0.00s)
panic: interface conversion: interface {} is []uint8, not uint8 [recovered, repanicked]

goroutine 21 [running]:
testing.tRunner.func1.2({0xb0e0e0, 0xc0002034d0})
	/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1872 +0x237
testing.tRunner.func1()
	/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1875 +0x35b
panic({0xb0e0e0?, 0xc0002034d0?})
	/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/runtime/panic.go:783 +0x132
github.com/carverauto/serviceradar/pkg/checker/snmp.convertOctetString({{0xab1040, 0xc000124c78}, {0xbfb1bc, 0x12}, 0x4})
	/home/user/serviceradar/pkg/checker/snmp/client.go:296 +0x69
github.com/carverauto/serviceradar/pkg/checker/snmp.(*SNMPClientImpl).convertVariable(0xc0001136a0?, {{0xab1040, 0xc000124c78}, {0xbfb1bc, 0x12}, 0x4})
	/home/user/serviceradar/pkg/checker/snmp/client.go:248 +0x5b5
github.com/carverauto/serviceradar/pkg/checker/snmp.TestConvertOctetStringTypeBug(0xc000100fc0)
	/home/user/serviceradar/pkg/checker/snmp/client_bug_test.go:22 +0xdc
testing.tRunner(0xc000100fc0, 0xc2f5e0)
	/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1934 +0xea
created by testing.(*T).Run in goroutine 1
	/root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1997 +0x465
FAIL	github.com/carverauto/serviceradar/pkg/checker/snmp	0.007s
FAIL

The panic message confirms: interface conversion: interface {} is []uint8, not uint8

Full context

The SNMP client is part of ServiceRadar's monitoring system. The call chain is:

SNMPService (pkg/checker/snmp/service.go) - The main service that manages SNMP monitoring targets
SNMPCollector (pkg/checker/snmp/collector.go) - Periodically polls SNMP devices by calling client.Get(oids)
SNMPClientImpl.Get() (pkg/checker/snmp/client.go line 107) - Executes SNMP queries using the gosnmp library
SNMPClientImpl.convertVariable() (pkg/checker/snmp/client.go line 206) - Converts raw gosnmp responses to Go types
convertOctetString() / convertObjectDescription() (pkg/checker/snmp/client.go lines 267, 295) - Where the bug occurs

When monitoring any SNMP device, the collector will typically query basic system information including:

sysDescr (1.3.6.1.2.1.1.1.0) - System description (OctetString)
sysContact (1.3.6.1.2.1.1.4.0) - System contact (OctetString)
sysName (1.3.6.1.2.1.1.5.0) - System name (OctetString)
sysLocation (1.3.6.1.2.1.1.6.0) - System location (OctetString)

All of these fundamental OIDs return OctetString types, so any monitoring configuration that includes these OIDs will trigger the panic. The panic crashes the collector goroutine, preventing further data collection from the affected target. If error handling doesn't properly recover from the panic, it could crash the entire monitoring service.

External documentation

gosnmp package documentation

// Example from documentation showing correct usage:
for _, variable := range result.Variables {
    switch variable.Type {
    case gosnmp.OctetString:
        bytes := variable.Value.([]byte)
        fmt.Printf("string: %s\n", string(bytes))
    }
}

SNMP sysDescr OID reference

OID: 1.3.6.1.2.1.1.1
Name: sysDescr
Type: DisplayString (OctetString)
Description: A textual description of the entity. This value should include
the full name and version identification of the system's hardware type,
software operating-system, and networking software.

Why has this bug gone undetected?

This bug has gone undetected for several reasons:

Limited test coverage: The existing tests in the SNMP package use mocks extensively. The collector_test.go file only tests the collector with mock SNMP clients, never exercising the actual type conversion code paths with real gosnmp PDU structures.
No integration tests with real SNMP data: While there is a file named snmp_integration_test.go, it likely doesn't test against live SNMP devices or use real OctetString values from the gosnmp library. The type conversion functions have never been tested with actual gosnmp.SnmpPDU values that contain OctetString types.
Uncommon data type in testing: String-type OIDs (OctetString, ObjectDescription) may not have been included in the test scenarios. Developers might have focused testing on numeric metrics (Counter32, Gauge32, Integer) which are more commonly used for monitoring metrics like CPU, memory, and bandwidth.
Recent code addition: The git history shows this code went through multiple linter fixes (more linter fixes, fixing linter issues in client.go, etc.). The conversion functions may have been added recently or refactored during linting cleanup, potentially introducing the bug without adequate testing of the changes.
Development/staging environment limitations: The development or staging environment might not have actual SNMP devices configured, or the test configurations might not include the common system OIDs (sysDescr, sysName, etc.) that would trigger this bug.
Collector-level type handling: Looking at collector.go line 377, the collector has its own convertString function that correctly handles []byte values. This suggests that in some code paths, the data might flow through alternative conversion logic that masks this bug, or the affected code paths simply haven't been exercised in production yet.

Recommended fix

Change both functions to correctly handle byte slices:

func convertObjectDescription(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.([]byte)) // <-- FIX 🟢
}

func convertOctetString(variable gosnmp.SnmpPDU) interface{} {
	return string(variable.Value.([]byte)) // <-- FIX 🟢
}

Additionally, consider adding error handling for robustness:

func convertOctetString(variable gosnmp.SnmpPDU) interface{} {
	if bytes, ok := variable.Value.([]byte); ok {
		return string(bytes)
	}
	// Fallback for unexpected types
	return fmt.Sprintf("%v", variable.Value)
}

Imported from GitHub. Original GitHub issue: #2154 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/issues/2154 Original created: 2025-12-16T05:19:17Z --- # Summary - **Context**: The SNMP client in `pkg/checker/snmp/client.go` converts SNMP PDU variables returned from the gosnmp library into Go types for use in the monitoring system. - **Bug**: The `convertOctetString` and `convertObjectDescription` functions incorrectly attempt to type-assert `variable.Value` as a single `byte` instead of a byte slice `[]byte`. - **Actual vs. expected**: The gosnmp library returns `[]byte` for OctetString and ObjectDescription types, but the code tries to convert to a single `byte`, causing a panic. - **Impact**: Any SNMP query that retrieves OctetString or ObjectDescription values (including the most common OID `sysDescr` used for device identification) will crash the application with a panic. # Code with bug ```go func convertObjectDescription(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.(byte)) // <-- BUG 🔴 Should be []byte, not byte } ``` ```go func convertOctetString(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.(byte)) // <-- BUG 🔴 Should be []byte, not byte } ``` # Evidence ## Example The SNMP OID `1.3.6.1.2.1.1.1.0` (sysDescr - system description) is one of the most fundamental SNMP OIDs used to identify network devices. This OID returns a DisplayString type, which is encoded as an OctetString in SNMP. When the gosnmp library queries this OID, it returns: ```go gosnmp.SnmpPDU{ Name: ".1.3.6.1.2.1.1.1.0", Type: gosnmp.OctetString, Value: []byte("Linux foo.example.lan 2.6.32-573.1.1.v6.i686..."), // This is []byte } ``` The code attempts: ```go string(variable.Value.(byte)) ``` This type assertion fails because: - Actual type: `[]byte` (also known as `[]uint8`) - Expected by code: `byte` (also known as `uint8`) The result is a panic: `interface conversion: interface {} is []uint8, not uint8` ## Inconsistency with API documentation ### Reference API documentation From the gosnmp documentation at https://pkg.go.dev/github.com/gosnmp/gosnmp: > the Value of each variable returned by Get() implements interface{}. You could do a type switch... Example from the documentation: ```go for _, variable := range result.Variables { switch variable.Type { case gosnmp.OctetString: bytes := variable.Value.([]byte) fmt.Printf("string: %s\n", string(bytes)) } } ``` The documentation clearly shows that OctetString values should be type-asserted to `[]byte`, not `byte`. ### Current API usage `pkg/checker/snmp/client.go` lines 268 and 296: ```go func convertObjectDescription(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.(byte)) } func convertOctetString(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.(byte)) } ``` ### Contradiction The code attempts to convert `variable.Value` to a single `byte`, but the gosnmp library's API documentation and implementation clearly shows that OctetString and ObjectDescription values are returned as `[]byte` (byte slices), not single bytes. ## Inconsistency within the codebase ### Reference code `pkg/checker/snmp/collector.go` lines 376-386: ```go // convertString converts a string value to a string. func (*SNMPCollector) convertString(value interface{}) (string, error) { switch v := value.(type) { case []byte: return string(v), nil case string: return v, nil default: return "", fmt.Errorf("%w %T", ErrInvalidStringType, value) } } ``` ### Current code `pkg/checker/snmp/client.go` lines 295-297: ```go func convertOctetString(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.(byte)) } ``` ### Comparison The collector correctly handles string conversion by accepting `[]byte` and converting it to `string`. The `convertString` function in the collector demonstrates the correct understanding that SNMP string values come as byte slices. However, the `convertOctetString` function in the client contradicts this by attempting to convert a single `byte`, which is inconsistent with both the gosnmp API and the codebase's own handling of similar data types. ## Failing test ### Test script ```go package snmp import ( "testing" "github.com/gosnmp/gosnmp" ) // TestConvertOctetStringTypeBug tests the bug where convertOctetString // incorrectly tries to convert Value to a single byte instead of []byte func TestConvertOctetStringTypeBug(t *testing.T) { client := &SNMPClientImpl{} // This is what gosnmp actually returns for OctetString types variable := gosnmp.SnmpPDU{ Name: ".1.3.6.1.2.1.1.1.0", Type: gosnmp.OctetString, Value: []byte("Test SNMP String"), } // This should work but will panic due to the bug _, err := client.convertVariable(variable) if err != nil { t.Fatalf("convertVariable failed: %v", err) } } // TestConvertObjectDescriptionTypeBug tests the bug where convertObjectDescription // incorrectly tries to convert Value to a single byte instead of []byte func TestConvertObjectDescriptionTypeBug(t *testing.T) { client := &SNMPClientImpl{} // ObjectDescription should also be []byte variable := gosnmp.SnmpPDU{ Name: ".1.3.6.1.2.1.1.2.0", Type: gosnmp.ObjectDescription, Value: []byte("Object Description"), } // This should work but will panic due to the bug _, err := client.convertVariable(variable) if err != nil { t.Fatalf("convertVariable failed: %v", err) } } ``` ### Test output ``` === RUN TestConvertOctetStringTypeBug --- FAIL: TestConvertOctetStringTypeBug (0.00s) panic: interface conversion: interface {} is []uint8, not uint8 [recovered, repanicked] goroutine 21 [running]: testing.tRunner.func1.2({0xb0e0e0, 0xc0002034d0}) /root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1872 +0x237 testing.tRunner.func1() /root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1875 +0x35b panic({0xb0e0e0?, 0xc0002034d0?}) /root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/runtime/panic.go:783 +0x132 github.com/carverauto/serviceradar/pkg/checker/snmp.convertOctetString({{0xab1040, 0xc000124c78}, {0xbfb1bc, 0x12}, 0x4}) /home/user/serviceradar/pkg/checker/snmp/client.go:296 +0x69 github.com/carverauto/serviceradar/pkg/checker/snmp.(*SNMPClientImpl).convertVariable(0xc0001136a0?, {{0xab1040, 0xc000124c78}, {0xbfb1bc, 0x12}, 0x4}) /home/user/serviceradar/pkg/checker/snmp/client.go:248 +0x5b5 github.com/carverauto/serviceradar/pkg/checker/snmp.TestConvertOctetStringTypeBug(0xc000100fc0) /home/user/serviceradar/pkg/checker/snmp/client_bug_test.go:22 +0xdc testing.tRunner(0xc000100fc0, 0xc2f5e0) /root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1934 +0xea created by testing.(*T).Run in goroutine 1 /root/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/testing/testing.go:1997 +0x465 FAIL github.com/carverauto/serviceradar/pkg/checker/snmp 0.007s FAIL ``` The panic message confirms: `interface conversion: interface {} is []uint8, not uint8` # Full context The SNMP client is part of ServiceRadar's monitoring system. The call chain is: 1. **SNMPService** (`pkg/checker/snmp/service.go`) - The main service that manages SNMP monitoring targets 2. **SNMPCollector** (`pkg/checker/snmp/collector.go`) - Periodically polls SNMP devices by calling `client.Get(oids)` 3. **SNMPClientImpl.Get()** (`pkg/checker/snmp/client.go` line 107) - Executes SNMP queries using the gosnmp library 4. **SNMPClientImpl.convertVariable()** (`pkg/checker/snmp/client.go` line 206) - Converts raw gosnmp responses to Go types 5. **convertOctetString()** / **convertObjectDescription()** (`pkg/checker/snmp/client.go` lines 267, 295) - Where the bug occurs When monitoring any SNMP device, the collector will typically query basic system information including: - `sysDescr` (1.3.6.1.2.1.1.1.0) - System description (OctetString) - `sysContact` (1.3.6.1.2.1.1.4.0) - System contact (OctetString) - `sysName` (1.3.6.1.2.1.1.5.0) - System name (OctetString) - `sysLocation` (1.3.6.1.2.1.1.6.0) - System location (OctetString) All of these fundamental OIDs return OctetString types, so any monitoring configuration that includes these OIDs will trigger the panic. The panic crashes the collector goroutine, preventing further data collection from the affected target. If error handling doesn't properly recover from the panic, it could crash the entire monitoring service. ## External documentation - [gosnmp package documentation](https://pkg.go.dev/github.com/gosnmp/gosnmp) ```go // Example from documentation showing correct usage: for _, variable := range result.Variables { switch variable.Type { case gosnmp.OctetString: bytes := variable.Value.([]byte) fmt.Printf("string: %s\n", string(bytes)) } } ``` - [SNMP sysDescr OID reference](https://oidref.com/1.3.6.1.2.1.1.1) ``` OID: 1.3.6.1.2.1.1.1 Name: sysDescr Type: DisplayString (OctetString) Description: A textual description of the entity. This value should include the full name and version identification of the system's hardware type, software operating-system, and networking software. ``` # Why has this bug gone undetected? This bug has gone undetected for several reasons: 1. **Limited test coverage**: The existing tests in the SNMP package use mocks extensively. The `collector_test.go` file only tests the collector with mock SNMP clients, never exercising the actual type conversion code paths with real gosnmp PDU structures. 2. **No integration tests with real SNMP data**: While there is a file named `snmp_integration_test.go`, it likely doesn't test against live SNMP devices or use real OctetString values from the gosnmp library. The type conversion functions have never been tested with actual gosnmp.SnmpPDU values that contain OctetString types. 3. **Uncommon data type in testing**: String-type OIDs (OctetString, ObjectDescription) may not have been included in the test scenarios. Developers might have focused testing on numeric metrics (Counter32, Gauge32, Integer) which are more commonly used for monitoring metrics like CPU, memory, and bandwidth. 4. **Recent code addition**: The git history shows this code went through multiple linter fixes (`more linter fixes`, `fixing linter issues in client.go`, etc.). The conversion functions may have been added recently or refactored during linting cleanup, potentially introducing the bug without adequate testing of the changes. 5. **Development/staging environment limitations**: The development or staging environment might not have actual SNMP devices configured, or the test configurations might not include the common system OIDs (sysDescr, sysName, etc.) that would trigger this bug. 6. **Collector-level type handling**: Looking at `collector.go` line 377, the collector has its own `convertString` function that correctly handles `[]byte` values. This suggests that in some code paths, the data might flow through alternative conversion logic that masks this bug, or the affected code paths simply haven't been exercised in production yet. # Recommended fix Change both functions to correctly handle byte slices: ```go func convertObjectDescription(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.([]byte)) // <-- FIX 🟢 } func convertOctetString(variable gosnmp.SnmpPDU) interface{} { return string(variable.Value.([]byte)) // <-- FIX 🟢 } ``` Additionally, consider adding error handling for robustness: ```go func convertOctetString(variable gosnmp.SnmpPDU) interface{} { if bytes, ok := variable.Value.([]byte); ok { return string(bytes) } // Fallback for unexpected types return fmt.Sprintf("%v", variable.Value) } ```

mfreeman451 commented

2026-03-28 04:27:30 +00:00

Author

Owner

Imported GitHub comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/issues/2154#issuecomment-3662493126
Original created: 2025-12-16T21:39:42Z

closing as completed

Imported GitHub comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/issues/2154#issuecomment-3662493126 Original created: 2025-12-16T21:39:42Z --- closing as completed