From fe908b8599fcdbe7d454f46b3dffe9b0ae659abd Mon Sep 17 00:00:00 2001 From: John Schember Date: Tue, 3 Nov 2026 15:14:52 -0500 Subject: [PATCH] video: bounds check motion compensation before calling the copy backends --- video.go | 26 +++++++++++- video_amd64.go | 3 ++ video_arm64.go | 3 ++ video_noasm.go | 3 ++ video_test.go | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 1 deletion(-) diff --git a/video.go b/video.go index b840866..87ad943 100644 --- a/video.go +++ b/video.go @@ -514,7 +514,7 @@ func (v *Video) decodeMacroblock() { v.mbRow = v.macroblockAddress / v.mbWidth v.mbCol = v.macroblockAddress % v.mbWidth - if v.mbCol >= v.mbWidth || v.mbRow >= v.mbHeight { + if v.macroblockAddress < 0 || v.mbCol >= v.mbWidth || v.mbRow >= v.mbHeight { return // corrupt stream } @@ -605,6 +605,30 @@ func (v *Video) decodeMotionVector(rSize, motion int) int { return motion } +// macroblockInBounds reports whether motion compensation stays inside the frame planes. +func macroblockInBounds(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth int, s, d *Frame) bool { + if !blockInBounds(s.Y.Data, d.Y.Data, lumaWidth, 16, motionH, motionV, mbRow, mbCol) { + return false + } + cmH := motionH / 2 + cmV := motionV / 2 + return blockInBounds(s.Cb.Data, d.Cb.Data, chromaWidth, 8, cmH, cmV, mbRow, mbCol) && + blockInBounds(s.Cr.Data, d.Cr.Data, chromaWidth, 8, cmH, cmV, mbRow, mbCol) +} + +// blockInBounds allows reads into the padding after a plane, but not writes. +func blockInBounds(src, dst []byte, stride, size, motionH, motionV, mbRow, mbCol int) bool { + if mbRow < 0 || mbCol < 0 || mbCol*size+size > stride { + return false + } + di := mbRow*size*stride + mbCol*size + if di+(size-1)*stride+size > len(dst) { + return false + } + si := (mbRow*size+(motionV>>1))*stride + mbCol*size + (motionH >> 1) + return si >= 0 && si+(size-1+(motionV&1))*stride+size+(motionH&1) <= cap(src) +} + func (v *Video) predictMacroblock() { fwH := v.motionForward.H fwV := v.motionForward.V diff --git a/video_amd64.go b/video_amd64.go index ffefb23..8783d24 100644 --- a/video_amd64.go +++ b/video_amd64.go @@ -9,6 +9,9 @@ func copyMacroblockSSE2(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth i func copyMacroblockAVX2(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth int, s, d *Frame) func copyMacroblock(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth int, s, d *Frame) { + if !macroblockInBounds(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth, s, d) { + return // corrupt stream + } if isAVX2 { copyMacroblockAVX2(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth, s, d) } else { diff --git a/video_arm64.go b/video_arm64.go index 30d78b4..79dd1e8 100644 --- a/video_arm64.go +++ b/video_arm64.go @@ -6,5 +6,8 @@ package mpeg func copyMacroblockNEON(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth int, s, d *Frame) func copyMacroblock(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth int, s, d *Frame) { + if !macroblockInBounds(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth, s, d) { + return // corrupt stream + } copyMacroblockNEON(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth, s, d) } diff --git a/video_noasm.go b/video_noasm.go index c085b64..c1d2d20 100644 --- a/video_noasm.go +++ b/video_noasm.go @@ -26,6 +26,9 @@ func bilinAvg(a, b, c, d uint64) uint64 { } func copyMacroblock(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth int, s, d *Frame) { + if !macroblockInBounds(motionH, motionV, mbRow, mbCol, lumaWidth, chromaWidth, s, d) { + return // corrupt stream + } hp := motionH >> 1 vp := motionV >> 1 lsi := ((mbRow<<4)+vp)*lumaWidth + (mbCol << 4) + hp diff --git a/video_test.go b/video_test.go index f6f9314..94624b8 100644 --- a/video_test.go +++ b/video_test.go @@ -102,6 +102,114 @@ func runParitySweep(t *testing.T, fn copyMacroblockFunc) { } } +// newGuardedFrame returns a frame with each plane surrounded by mark bytes. +func newGuardedFrame(lumaWidth, chromaWidth, fill int, mark byte) (*Frame, [][]byte) { + const guard = 4096 + f := newTestFrame(lumaWidth, chromaWidth, fill) + var backing [][]byte + carve := func(p *Plane) { + n := len(p.Data) + b := make([]byte, guard+n+guard) + for i := range b { + b[i] = mark + } + copy(b[guard:], p.Data) + p.Data = b[guard : guard+n : guard+n] + backing = append(backing, b) + } + carve(&f.Y) + carve(&f.Cb) + carve(&f.Cr) + return f, backing +} + +func guardsIntact(backing [][]byte, mark byte) bool { + const guard = 4096 + for _, b := range backing { + for i := 0; i < guard; i++ { + if b[i] != mark || b[len(b)-1-i] != mark { + return false + } + } + } + return true +} + +func TestCopyMacroblockOutOfBounds(t *testing.T) { + const lumaWidth, chromaWidth = 64, 32 + const mbs = lumaWidth / 16 + + cases := []struct { + name string + motionH, motionV, mbRow, mbCol int + }{ + {"negative column", 0, 0, 0, -1}, + {"negative row", 0, 0, -1, 0}, + {"column past width", 0, 0, 0, mbs}, + {"row past height", 0, 0, mbs, 0}, + {"motion above frame", 0, -2048, 1, 1}, + {"motion below frame", 0, 2048, 1, 1}, + {"motion left of frame", -2048, 0, 0, 0}, + {"motion right of frame", 2048, 0, mbs - 1, mbs - 1}, + {"half-pel past bottom right", 1, 1, mbs - 1, mbs - 1}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + src, srcBacking := newGuardedFrame(lumaWidth, chromaWidth, 1, 0x5A) + dst, dstBacking := newGuardedFrame(lumaWidth, chromaWidth, 0, 0xA5) + want := newTestFrame(lumaWidth, chromaWidth, 0) + + copyMacroblock(c.motionH, c.motionV, c.mbRow, c.mbCol, lumaWidth, chromaWidth, src, dst) + + if !guardsIntact(dstBacking, 0xA5) || !guardsIntact(srcBacking, 0x5A) { + t.Fatal("memory outside the frame planes was modified") + } + if string(dst.Y.Data) != string(want.Y.Data) || + string(dst.Cb.Data) != string(want.Cb.Data) || + string(dst.Cr.Data) != string(want.Cr.Data) { + t.Fatal("destination modified by an out of bounds macroblock") + } + }) + } +} + +// TestCopyMacroblockPaddedEdge checks half-pel reads into the frame padding are allowed. +func TestCopyMacroblockPaddedEdge(t *testing.T) { + const mbWidth, mbHeight = 4, 3 + v := &Video{ + width: mbWidth * 16, + height: mbHeight * 16, + lumaWidth: mbWidth * 16, + lumaHeight: mbHeight * 16, + chromaWidth: mbWidth * 8, + chromaHeight: mbHeight * 8, + } + var src, got, want Frame + v.initFrame(&src) + v.initFrame(&got) + v.initFrame(&want) + base := src.Y.Data[:cap(src.Y.Data)] + for i := range base { + base[i] = byte(i*131 + 7) + } + + wide := src + wide.Y.Data = src.Y.Data[:cap(src.Y.Data)] + wide.Cb.Data = src.Cb.Data[:cap(src.Cb.Data)] + wide.Cr.Data = src.Cr.Data[:cap(src.Cr.Data)] + + const motionH, motionV = 3, 3 + mbRow, mbCol := mbHeight-1, mbWidth-1 + copyMacroblock(motionH, motionV, mbRow, mbCol, v.lumaWidth, v.chromaWidth, &src, &got) + copyMacroblockRef(motionH, motionV, mbRow, mbCol, v.lumaWidth, v.chromaWidth, &wide, &want) + + if string(got.Y.Data) != string(want.Y.Data) || + string(got.Cb.Data) != string(want.Cb.Data) || + string(got.Cr.Data) != string(want.Cr.Data) { + t.Fatal("bottom right macroblock not predicted from the padded reference") + } +} + func benchmarkCopyMacroblock(b *testing.B, motionH, motionV int) { const lumaWidth, chromaWidth = 64, 32 src := newTestFrame(lumaWidth, chromaWidth, 1)