Skip to content

Commit

Permalink
sysenc: refuse to unmarshal into undersized dst
Browse files Browse the repository at this point in the history
Unmarshal currently doesn't check that unmarshaling actually
consumes to full buffer. This means that looking uint16 from an
uint32 value doesn't return an error.

Fixes #1157

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
  • Loading branch information
lmb committed Oct 20, 2023
1 parent e83be32 commit 7d2810c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
10 changes: 9 additions & 1 deletion internal/sysenc/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,15 @@ func Unmarshal(data interface{}, buf []byte) error {

rd.Reset(buf)

return binary.Read(rd, internal.NativeEndian, value)
if err := binary.Read(rd, internal.NativeEndian, value); err != nil {
return err
}

if rd.Len() != 0 {
return fmt.Errorf("unmarshaling %T doesn't consume all data", data)
}

return nil
}
}

Expand Down
13 changes: 11 additions & 2 deletions map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,15 @@ func TestBatchAPIHash(t *testing.T) {
}
}

func TestMapLookupKeyTooSmall(t *testing.T) {
m := createArray(t)
defer m.Close()

var small uint16
qt.Assert(t, m.Put(uint32(0), uint32(1234)), qt.IsNil)
qt.Assert(t, m.Lookup(uint32(0), &small), qt.IsNotNil)
}

func TestBatchAPIMapDelete(t *testing.T) {
if err := haveBatchAPI(); err != nil {
t.Skipf("batch api not available: %v", err)
Expand Down Expand Up @@ -1015,7 +1024,7 @@ func TestIterateEmptyMap(t *testing.T) {
entries := m.Iterate()

var key string
var value uint32
var value uint64
if entries.Next(&key, &value) != false {
t.Error("Empty hash should not be iterable")
}
Expand All @@ -1033,7 +1042,7 @@ func TestIterateEmptyMap(t *testing.T) {
m := makeMap(t, mapType)
entries := m.Iterate()
var key string
var value uint32
var value uint64
for entries.Next(&key, &value) {
// Some empty arrays like sockmap don't return any keys.
}
Expand Down

0 comments on commit 7d2810c

Please sign in to comment.