From 6a6c2c521eb6f25f289dc6eb4a907a2b5e16b05c Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 6 Mar 2026 07:36:54 +0000 Subject: [PATCH] userdata: fix out-of-bounds panic in Get The Get function previously attempted to slice the userdata buffer before verifying that the buffer was large enough to contain the declared length. This would cause a panic when processing truncated or malformed data. This patch adds a bounds check to ensure the remaining slice length is sufficient before performing the sub-slice operation. Added a regression test to verify that malformed TLV data is handled gracefully without panicking. Signed-off-by: Antonio Ojea --- userdata/userdata.go | 12 ++++--- userdata/userdata_test.go | 67 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/userdata/userdata.go b/userdata/userdata.go index 71b2be6f..fbdb0645 100644 --- a/userdata/userdata.go +++ b/userdata/userdata.go @@ -67,23 +67,25 @@ func Append(udata []byte, typ Type, data []byte) []byte { func Get(udata []byte, styp Type) []byte { for { + // Ensure we have at least the Type and Length bytes if len(udata) < 2 { break } typ := Type(udata[0]) length := int(udata[1]) + + if len(udata) < 2+length { + break + } + data := udata[2 : 2+length] if styp == typ { return data } - if len(udata) < 2+length { - break - } else { - udata = udata[2+length:] - } + udata = udata[2+length:] } return nil diff --git a/userdata/userdata_test.go b/userdata/userdata_test.go index a2ea94a3..cb6192b1 100644 --- a/userdata/userdata_test.go +++ b/userdata/userdata_test.go @@ -70,3 +70,70 @@ func TestUint32(t *testing.T) { t.Fatalf("id mismatch") } } + +func TestGetOutOfBounds(t *testing.T) { + tests := []struct { + name string + input []byte + styp userdata.Type + }{ + { + name: "TruncatedHeader", + input: []byte{byte(userdata.TypeComment)}, // Only 1 byte, needs 2 for T+L + styp: userdata.TypeComment, + }, + { + name: "DeclaredLengthTooLong", + input: []byte{byte(userdata.TypeComment), 10, 'h', 'i'}, // Declares 10, only provides 2 + styp: userdata.TypeComment, + }, + { + name: "EmptyInput", + input: []byte{}, + styp: userdata.TypeComment, + }, + { + name: "ValidHeaderButMissingValue", + input: []byte{byte(userdata.TypeComment), 1}, // Declares 1 byte, but slice ends + styp: userdata.TypeComment, + }, + { + name: "MultipleElementsSecondTruncatedHeader", + input: []byte{ + byte(userdata.TypeComment), 2, 'h', 'i', // Valid first element + byte(userdata.TypeEbtablesPolicy), // Truncated second element header + }, + styp: userdata.TypeEbtablesPolicy, + }, + { + name: "MultipleElementsSecondDeclaredLengthTooLong", + input: []byte{ + byte(userdata.TypeComment), 2, 'h', 'i', // Valid first element + byte(userdata.TypeEbtablesPolicy), 10, 'b', 'a', // Invalid second element + }, + styp: userdata.TypeEbtablesPolicy, + }, + { + name: "ExactLengthButIteratingFurther", + input: []byte{byte(userdata.TypeComment), 2, 'h', 'i'}, // Valid lengths + styp: userdata.TypeEbtablesPolicy, // Search for something not there + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // This test ensures the code does not panic. + // The defer/recover block catches a panic if the fix isn't working. + defer func() { + if r := recover(); r != nil { + t.Errorf("Get() panicked on input %x: %v", tc.input, r) + } + }() + + // Testing the wrapper which calls the underlying Get() + if _, ok := userdata.GetString(tc.input, tc.styp); ok { + t.Errorf("GetString() should have failed for malformed input %x", tc.input) + } + }) + } +}