From dc644eb6ac6e2eccda22e15039fbbfc43da0c8a4 Mon Sep 17 00:00:00 2001 From: Aidar Fattakhov Date: Fri, 22 Nov 2024 13:54:16 +0100 Subject: [PATCH 1/3] Fix PropExperimenter deserialization out-of-bounds read Signed-off-by: Aidar Fattakhov --- openflow15/group.go | 2 +- openflow15/openflow15.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openflow15/group.go b/openflow15/group.go index 1deb15d..e7af06c 100644 --- a/openflow15/group.go +++ b/openflow15/group.go @@ -162,7 +162,7 @@ func (g *GroupMod) UnmarshalBinary(data []byte) (err error) { g.CommandBucketId = binary.BigEndian.Uint32(data[n:]) n += 4 - for n < g.Header.Length { + for len(g.Buckets) < int(g.BucketArrayLen) && n < g.Header.Length { bkt := new(Bucket) err = bkt.UnmarshalBinary(data[n:]) if err != nil { diff --git a/openflow15/openflow15.go b/openflow15/openflow15.go index fd58f47..34d41d0 100644 --- a/openflow15/openflow15.go +++ b/openflow15/openflow15.go @@ -1516,7 +1516,7 @@ func (p *PropExperimenter) Len() uint16 { func (p *PropExperimenter) MarshalBinary() (data []byte, err error) { data = make([]byte, int(p.Len())) - p.Header.Length = 8 + uint16(len(p.Data)*4) + p.Header.Length = p.Header.Len() + 8 + uint16(len(p.Data)*4) b, err := p.Header.MarshalBinary() if err != nil { return @@ -1547,7 +1547,7 @@ func (p *PropExperimenter) UnmarshalBinary(data []byte) (err error) { p.ExpType = binary.BigEndian.Uint32(data[n:]) n += 4 - for n < p.Header.Length+p.Header.Len() { + for n < p.Header.Length { d := binary.BigEndian.Uint32(data[n:]) p.Data = append(p.Data, d) n += 4 From d212d41b24b0150ad61fd8eae6f7fbbcf4deecab Mon Sep 17 00:00:00 2001 From: Aidar Fattakhov Date: Thu, 5 Dec 2024 17:53:29 +0100 Subject: [PATCH 2/3] Remove unnecessary bound-check Signed-off-by: Aidar Fattakhov --- openflow15/group.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openflow15/group.go b/openflow15/group.go index e7af06c..243eb12 100644 --- a/openflow15/group.go +++ b/openflow15/group.go @@ -162,7 +162,7 @@ func (g *GroupMod) UnmarshalBinary(data []byte) (err error) { g.CommandBucketId = binary.BigEndian.Uint32(data[n:]) n += 4 - for len(g.Buckets) < int(g.BucketArrayLen) && n < g.Header.Length { + for len(g.Buckets) < int(g.BucketArrayLen) { bkt := new(Bucket) err = bkt.UnmarshalBinary(data[n:]) if err != nil { From a1582a84a80bfafe89f7d4a4918e1f76ad56a8a5 Mon Sep 17 00:00:00 2001 From: Aidar Fattakhov Date: Thu, 12 Dec 2024 16:57:36 +0100 Subject: [PATCH 3/3] Fix bucket boundcheck Signed-off-by: Aidar Fattakhov --- openflow15/group.go | 2 +- openflow15/openflow15.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/openflow15/group.go b/openflow15/group.go index 243eb12..20e5c4f 100644 --- a/openflow15/group.go +++ b/openflow15/group.go @@ -162,7 +162,7 @@ func (g *GroupMod) UnmarshalBinary(data []byte) (err error) { g.CommandBucketId = binary.BigEndian.Uint32(data[n:]) n += 4 - for len(g.Buckets) < int(g.BucketArrayLen) { + for n < g.BucketArrayLen+24 { bkt := new(Bucket) err = bkt.UnmarshalBinary(data[n:]) if err != nil { diff --git a/openflow15/openflow15.go b/openflow15/openflow15.go index 34d41d0..b1f0ba6 100644 --- a/openflow15/openflow15.go +++ b/openflow15/openflow15.go @@ -1509,8 +1509,6 @@ func (p *PropExperimenter) Len() uint16 { n += 8 l := uint16(len(p.Data) * 4) n += l - //n += uint16((8 - (l % 8)) % 8) // pad to make multiple of 8 - n += uint16(8 - (l % 8)) // pad to make multiple of 8 return n }