From 57179dff69a6501036f60dc63ee445cccfd5f24b Mon Sep 17 00:00:00 2001 From: Victor Gaydov Date: Tue, 12 Nov 2019 16:19:04 +0300 Subject: [PATCH] Fix memory corruption --- AUTHORS | 1 + decoder.go | 23 ++++++-- opus_test.go | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 5 deletions(-) diff --git a/AUTHORS b/AUTHORS index 1cf8b9f..034c511 100644 --- a/AUTHORS +++ b/AUTHORS @@ -6,3 +6,4 @@ Hraban Luyat Dejian Xu Tobias Wellnitz Elinor Natanzon +Victor Gaydov diff --git a/decoder.go b/decoder.go index a3760ce..132a3e9 100644 --- a/decoder.go +++ b/decoder.go @@ -28,6 +28,7 @@ type Decoder struct { // Same purpose as encoder struct mem []byte sample_rate int + channels int } // NewDecoder allocates a new Opus decoder and initializes it with the @@ -50,6 +51,7 @@ func (dec *Decoder) Init(sample_rate int, channels int) error { } size := C.opus_decoder_get_size(C.int(channels)) dec.sample_rate = sample_rate + dec.channels = channels dec.mem = make([]byte, size) dec.p = (*C.OpusDecoder)(unsafe.Pointer(&dec.mem[0])) errno := C.opus_decoder_init( @@ -74,12 +76,15 @@ func (dec *Decoder) Decode(data []byte, pcm []int16) (int, error) { if len(pcm) == 0 { return 0, fmt.Errorf("opus: target buffer empty") } + if cap(pcm)%dec.channels != 0 { + return 0, fmt.Errorf("opus: target buffer capacity must be multiple of channels") + } n := int(C.opus_decode( dec.p, (*C.uchar)(&data[0]), C.opus_int32(len(data)), (*C.opus_int16)(&pcm[0]), - C.int(cap(pcm)), + C.int(cap(pcm)/dec.channels), 0)) if n < 0 { return 0, Error(n) @@ -99,12 +104,15 @@ func (dec *Decoder) DecodeFloat32(data []byte, pcm []float32) (int, error) { if len(pcm) == 0 { return 0, fmt.Errorf("opus: target buffer empty") } + if cap(pcm)%dec.channels != 0 { + return 0, fmt.Errorf("opus: target buffer capacity must be multiple of channels") + } n := int(C.opus_decode_float( dec.p, (*C.uchar)(&data[0]), C.opus_int32(len(data)), (*C.float)(&pcm[0]), - C.int(cap(pcm)), + C.int(cap(pcm)/dec.channels), 0)) if n < 0 { return 0, Error(n) @@ -125,13 +133,15 @@ func (dec *Decoder) DecodeFEC(data []byte, pcm []int16) error { if len(pcm) == 0 { return fmt.Errorf("opus: target buffer empty") } - + if cap(pcm)%dec.channels != 0 { + return fmt.Errorf("opus: target buffer capacity must be multiple of channels") + } n := int(C.opus_decode( dec.p, (*C.uchar)(&data[0]), C.opus_int32(len(data)), (*C.opus_int16)(&pcm[0]), - C.int(cap(pcm)), + C.int(cap(pcm)/dec.channels), 1)) if n < 0 { return Error(n) @@ -152,12 +162,15 @@ func (dec *Decoder) DecodeFECFloat32(data []byte, pcm []float32) error { if len(pcm) == 0 { return fmt.Errorf("opus: target buffer empty") } + if cap(pcm)%dec.channels != 0 { + return fmt.Errorf("opus: target buffer capacity must be multiple of channels") + } n := int(C.opus_decode_float( dec.p, (*C.uchar)(&data[0]), C.opus_int32(len(data)), (*C.float)(&pcm[0]), - C.int(cap(pcm)), + C.int(cap(pcm)/dec.channels), 1)) if n < 0 { return Error(n) diff --git a/opus_test.go b/opus_test.go index 645be57..53886ed 100644 --- a/opus_test.go +++ b/opus_test.go @@ -307,3 +307,153 @@ func TestStereo(t *testing.T) { } */ } + +func TestBufferSize(t *testing.T) { + const G4 = 391.995 + const SAMPLE_RATE = 48000 + const FRAME_SIZE_MS = 60 + const FRAME_SIZE = SAMPLE_RATE * FRAME_SIZE_MS / 1000 + const GUARD_SIZE = 100 + + checkGuardInt16 := func(t *testing.T, s []int16) { + for n := range s { + if s[n] != 0 { + t.Fatal("Memory corruption detected") + } + } + } + + checkGuardFloat32 := func(t *testing.T, s []float32) { + for n := range s { + if s[n] != 0 { + t.Fatal("Memory corruption detected") + } + } + } + + checkResult := func(t *testing.T, n int, err error, expectOK bool) { + if expectOK { + if err != nil { + t.Fatalf("Couldn't decode data: %v", err) + } + if n != FRAME_SIZE { + t.Fatalf("Length mismatch: %d samples in, %d out", FRAME_SIZE, n) + } + } else { + if err == nil { + t.Fatalf("Expected decode failure, but it succeeded") + } + } + } + + encodeFrame := func(t *testing.T) []byte { + pcm := make([]int16, FRAME_SIZE) + enc, err := NewEncoder(SAMPLE_RATE, 1, AppVoIP) + if err != nil || enc == nil { + t.Fatalf("Error creating new encoder: %v", err) + } + addSine(pcm, SAMPLE_RATE, G4) + data := make([]byte, 1000) + n, err := enc.Encode(pcm, data) + if err != nil { + t.Fatalf("Couldn't encode data: %v", err) + } + return data[:n] + } + + createDecoder := func(t *testing.T) *Decoder { + dec, err := NewDecoder(SAMPLE_RATE, 1) + if err != nil || dec == nil { + t.Fatalf("Error creating new decoder: %v", err) + } + return dec + } + + decodeInt16 := func(t *testing.T, data []byte, decodeSize int, expectOK bool) { + dec := createDecoder(t) + decodedMem := make([]int16, decodeSize+GUARD_SIZE*2) + decodedRef := decodedMem[GUARD_SIZE : GUARD_SIZE+decodeSize : GUARD_SIZE+decodeSize] + n, err := dec.Decode(data, decodedRef) + checkGuardInt16(t, decodedMem[:GUARD_SIZE]) + checkGuardInt16(t, decodedMem[decodeSize+GUARD_SIZE:]) + checkResult(t, n, err, expectOK) + } + + decodeFloat32 := func(t *testing.T, data []byte, decodeSize int, expectOK bool) { + dec := createDecoder(t) + decodedMem := make([]float32, decodeSize+GUARD_SIZE*2) + decodedRef := decodedMem[GUARD_SIZE : GUARD_SIZE+decodeSize : GUARD_SIZE+decodeSize] + n, err := dec.DecodeFloat32(data, decodedRef) + checkGuardFloat32(t, decodedMem[:GUARD_SIZE]) + checkGuardFloat32(t, decodedMem[decodeSize+GUARD_SIZE:]) + checkResult(t, n, err, expectOK) + } + + decodeFecInt16 := func(t *testing.T, data []byte, decodeSize int, expectOK bool) { + dec := createDecoder(t) + decodedMem := make([]int16, decodeSize+GUARD_SIZE*2) + decodedRef := decodedMem[GUARD_SIZE : GUARD_SIZE+decodeSize : GUARD_SIZE+decodeSize] + err := dec.DecodeFEC(data, decodedRef) + checkGuardInt16(t, decodedMem[:GUARD_SIZE]) + checkGuardInt16(t, decodedMem[decodeSize+GUARD_SIZE:]) + checkResult(t, FRAME_SIZE, err, expectOK) + } + + decodeFecFloat32 := func(t *testing.T, data []byte, decodeSize int, expectOK bool) { + dec := createDecoder(t) + decodedMem := make([]float32, decodeSize+GUARD_SIZE*2) + decodedRef := decodedMem[GUARD_SIZE : GUARD_SIZE+decodeSize : GUARD_SIZE+decodeSize] + err := dec.DecodeFECFloat32(data, decodedRef) + checkGuardFloat32(t, decodedMem[:GUARD_SIZE]) + checkGuardFloat32(t, decodedMem[decodeSize+GUARD_SIZE:]) + checkResult(t, FRAME_SIZE, err, expectOK) + } + + t.Run("smaller-buffer-int16", func(t *testing.T) { + decodeInt16(t, encodeFrame(t), FRAME_SIZE-1, false) + }) + + t.Run("smaller-buffer-float32", func(t *testing.T) { + decodeFloat32(t, encodeFrame(t), FRAME_SIZE-1, false) + }) + + t.Run("smaller-buffer-int16-fec", func(t *testing.T) { + decodeFecFloat32(t, encodeFrame(t), FRAME_SIZE-1, false) + }) + + t.Run("smaller-buffer-float32-fec", func(t *testing.T) { + decodeFecFloat32(t, encodeFrame(t), FRAME_SIZE-1, false) + }) + + t.Run("exact-buffer-int16", func(t *testing.T) { + decodeInt16(t, encodeFrame(t), FRAME_SIZE, true) + }) + + t.Run("exact-buffer-float32", func(t *testing.T) { + decodeFloat32(t, encodeFrame(t), FRAME_SIZE, true) + }) + + t.Run("exact-buffer-int16-fec", func(t *testing.T) { + decodeFecInt16(t, encodeFrame(t), FRAME_SIZE, true) + }) + + t.Run("exact-buffer-float32-fec", func(t *testing.T) { + decodeFecFloat32(t, encodeFrame(t), FRAME_SIZE, true) + }) + + t.Run("larger-buffer-int16", func(t *testing.T) { + decodeInt16(t, encodeFrame(t), FRAME_SIZE+1, true) + }) + + t.Run("larger-buffer-float32", func(t *testing.T) { + decodeFloat32(t, encodeFrame(t), FRAME_SIZE+1, true) + }) + + t.Run("larger-buffer-int16-fec", func(t *testing.T) { + decodeFecInt16(t, encodeFrame(t), FRAME_SIZE+1, false) + }) + + t.Run("larger-buffer-float32-fec", func(t *testing.T) { + decodeFecFloat32(t, encodeFrame(t), FRAME_SIZE+1, false) + }) +} -- 2.48.1