From de95da9bf1c98b80368cf643e3b7da45f3ca108b Mon Sep 17 00:00:00 2001
From: Gregory Maxwell <greg@xiph.org>
Date: Sat, 27 Oct 2012 13:42:48 -0400
Subject: [PATCH] Fix several issues with multistream argument validation.
As reported by Mark Warner opus_multistream_*_create were depending on
the behavior of malloc(0) in order to correctly report some kinds of
argument errors. Bad arguments could be incorrectly reported as
allocation failures. This changes multistream to explicitly check the
arguments like the single stream _create functions. The unit tests were
enough to catch this on systems where malloc(0) returns NULL but didn't
catch it on other systems because the later _init call would catch the
bad arguments and trigger the correct error if and only if the malloc
didn't return a null pointer.
In multistream_encoder_init failures of the internal non-multistream
init calls were not being caught and propagated. Decode didn't have
this problem. This propagates the errors and adds additional tests
(the multistream encoder api is sill under tested) that would have
detected this error.
Plus add some stronger tests for things like error==NULL for the _create
functions that take a pointer for error output.
---
src/opus_multistream.c | 30 ++++--
tests/test_opus_api.c | 209 +++++++++++++++++++++------------------
tests/test_opus_encode.c | 23 +++++
3 files changed, 161 insertions(+), 101 deletions(-)
diff --git a/src/opus_multistream.c b/src/opus_multistream.c
index 646e9b64e..9c427b5d9 100644
--- a/src/opus_multistream.c
+++ b/src/opus_multistream.c
@@ -159,7 +159,7 @@ int opus_multistream_encoder_init(
{
int coupled_size;
int mono_size;
- int i;
+ int i, ret;
char *ptr;
if ((channels>255) || (channels<1) || (coupled_streams>streams) ||
@@ -180,12 +180,14 @@ int opus_multistream_encoder_init(
for (i=0;i<st->layout.nb_coupled_streams;i++)
{
- opus_encoder_init((OpusEncoder*)ptr, Fs, 2, application);
+ ret = opus_encoder_init((OpusEncoder*)ptr, Fs, 2, application);
+ if(ret!=OPUS_OK)return ret;
ptr += align(coupled_size);
}
for (;i<st->layout.nb_streams;i++)
{
- opus_encoder_init((OpusEncoder*)ptr, Fs, 1, application);
+ ret = opus_encoder_init((OpusEncoder*)ptr, Fs, 1, application);
+ if(ret!=OPUS_OK)return ret;
ptr += align(mono_size);
}
return OPUS_OK;
@@ -202,7 +204,15 @@ OpusMSEncoder *opus_multistream_encoder_create(
)
{
int ret;
- OpusMSEncoder *st = (OpusMSEncoder *)opus_alloc(opus_multistream_encoder_get_size(streams, coupled_streams));
+ OpusMSEncoder *st;
+ if ((channels>255) || (channels<1) || (coupled_streams>streams) ||
+ (coupled_streams+streams>255) || (streams<1) || (coupled_streams<0))
+ {
+ if (error)
+ *error = OPUS_BAD_ARG;
+ return NULL;
+ }
+ st = (OpusMSEncoder *)opus_alloc(opus_multistream_encoder_get_size(streams, coupled_streams));
if (st==NULL)
{
if (error)
@@ -649,7 +659,15 @@ OpusMSDecoder *opus_multistream_decoder_create(
)
{
int ret;
- OpusMSDecoder *st = (OpusMSDecoder *)opus_alloc(opus_multistream_decoder_get_size(streams, coupled_streams));
+ OpusMSDecoder *st;
+ if ((channels>255) || (channels<1) || (coupled_streams>streams) ||
+ (coupled_streams+streams>255) || (streams<1) || (coupled_streams<0))
+ {
+ if (error)
+ *error = OPUS_BAD_ARG;
+ return NULL;
+ }
+ st = (OpusMSDecoder *)opus_alloc(opus_multistream_decoder_get_size(streams, coupled_streams));
if (st==NULL)
{
if (error)
@@ -665,8 +683,6 @@ OpusMSDecoder *opus_multistream_decoder_create(
st = NULL;
}
return st;
-
-
}
typedef void (*opus_copy_channel_out_func)(
diff --git a/tests/test_opus_api.c b/tests/test_opus_api.c
index 187d8c381..fa62e6496 100644
--- a/tests/test_opus_api.c
+++ b/tests/test_opus_api.c
@@ -126,6 +126,9 @@ opus_int32 test_dec_api(void)
dec = opus_decoder_create(fs, c, &err);
if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
cfgs++;
+ dec = opus_decoder_create(fs, c, 0);
+ if(dec!=NULL)test_failed();
+ cfgs++;
dec=malloc(opus_decoder_get_size(2));
if(dec==NULL)test_failed();
err = opus_decoder_init(dec,fs,c);
@@ -366,6 +369,9 @@ opus_int32 test_msdec_api(void)
dec = opus_multistream_decoder_create(fs, c, 1, c-1, mapping, &err);
if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
cfgs++;
+ dec = opus_multistream_decoder_create(fs, c, 1, c-1, mapping, 0);
+ if(dec!=NULL)test_failed();
+ cfgs++;
dec=malloc(opus_multistream_decoder_get_size(1,1));
if(dec==NULL)test_failed();
err = opus_multistream_decoder_init(dec,fs,c,1,c-1, mapping);
@@ -375,116 +381,128 @@ opus_int32 test_msdec_api(void)
}
}
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err==OPUS_OK || dec!=NULL)test_failed();
- cfgs++;
+ for(c=0;c<2;c++)
+ {
+ int *ret_err;
+ ret_err = c?0:&err;
- VG_UNDEF(&err,sizeof(err));
- mapping[0]=mapping[1]=0;
- dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_OK || dec==NULL)test_failed();
- cfgs++;
- opus_multistream_decoder_destroy(dec);
- cfgs++;
+ mapping[0]=0;
+ mapping[1]=1;
+ for(i=2;i<256;i++)VG_UNDEF(&mapping[i],sizeof(unsigned char));
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 1, 4, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_OK || dec==NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- err = opus_multistream_decoder_init(dec,48000, 1, 0, 0, mapping);
- if(err!=OPUS_BAD_ARG)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ mapping[0]=mapping[1]=0;
+ dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+ cfgs++;
+ opus_multistream_decoder_destroy(dec);
+ cfgs++;
- err = opus_multistream_decoder_init(dec,48000, 1, 1, -1, mapping);
- if(err!=OPUS_BAD_ARG)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 1, 4, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+ cfgs++;
- opus_multistream_decoder_destroy(dec);
- cfgs++;
+ err = opus_multistream_decoder_init(dec,48000, 1, 0, 0, mapping);
+ if(err!=OPUS_BAD_ARG)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 2, 1, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_OK || dec==NULL)test_failed();
- cfgs++;
- opus_multistream_decoder_destroy(dec);
- cfgs++;
+ err = opus_multistream_decoder_init(dec,48000, 1, 1, -1, mapping);
+ if(err!=OPUS_BAD_ARG)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 255, 255, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ opus_multistream_decoder_destroy(dec);
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, -1, 1, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 2, 1, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+ cfgs++;
+ opus_multistream_decoder_destroy(dec);
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 0, 1, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 255, 255, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 1, -1, 2, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, -1, 1, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 1, -1, -1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 0, 1, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 256, 255, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 1, -1, 2, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- dec = opus_multistream_decoder_create(48000, 256, 255, 0, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 1, -1, -1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- mapping[0]=255;
- mapping[1]=1;
- mapping[2]=2;
- dec = opus_multistream_decoder_create(48000, 3, 2, 0, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 256, 255, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- VG_UNDEF(&err,sizeof(err));
- mapping[0]=0;
- mapping[1]=0;
- mapping[2]=0;
- dec = opus_multistream_decoder_create(48000, 3, 2, 1, mapping, &err);
- VG_CHECK(&err,sizeof(err));
- if(err!=OPUS_OK || dec==NULL)test_failed();
- cfgs++;
- opus_multistream_decoder_destroy(dec);
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ dec = opus_multistream_decoder_create(48000, 256, 255, 0, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
- mapping[0]=0;
- mapping[1]=255;
- mapping[2]=1;
- mapping[3]=2;
- mapping[4]=3;
- dec = opus_multistream_decoder_create(48001, 5, 4, 1, mapping, 0);
- if(dec!=NULL)test_failed();
- cfgs++;
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ mapping[0]=255;
+ mapping[1]=1;
+ mapping[2]=2;
+ dec = opus_multistream_decoder_create(48000, 3, 2, 0, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
+
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ mapping[0]=0;
+ mapping[1]=0;
+ mapping[2]=0;
+ dec = opus_multistream_decoder_create(48000, 3, 2, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+ cfgs++;
+ opus_multistream_decoder_destroy(dec);
+ cfgs++;
+
+ VG_UNDEF(ret_err,sizeof(*ret_err));
+ mapping[0]=0;
+ mapping[1]=255;
+ mapping[2]=1;
+ mapping[3]=2;
+ mapping[4]=3;
+ dec = opus_multistream_decoder_create(48001, 5, 4, 1, mapping, ret_err);
+ if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+ if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+ cfgs++;
+ }
VG_UNDEF(&err,sizeof(err));
mapping[0]=0;
@@ -1061,6 +1079,9 @@ opus_int32 test_enc_api(void)
enc = opus_encoder_create(fs, c, OPUS_APPLICATION_VOIP, &err);
if(err!=OPUS_BAD_ARG || enc!=NULL)test_failed();
cfgs++;
+ enc = opus_encoder_create(fs, c, OPUS_APPLICATION_VOIP, 0);
+ if(enc!=NULL)test_failed();
+ cfgs++;
opus_encoder_destroy(enc);
enc=malloc(opus_encoder_get_size(2));
if(enc==NULL)test_failed();
diff --git a/tests/test_opus_encode.c b/tests/test_opus_encode.c
index a9a1c58b0..61e0dec18 100644
--- a/tests/test_opus_encode.c
+++ b/tests/test_opus_encode.c
@@ -144,6 +144,29 @@ int run_test1(int no_fuzz)
enc = opus_encoder_create(48000, 2, OPUS_APPLICATION_VOIP, &err);
if(err != OPUS_OK || enc==NULL)test_failed();
+ for(i=0;i<2;i++)
+ {
+ int *ret_err;
+ ret_err = i?0:&err;
+ MSenc = opus_multistream_encoder_create(8000, 2, 2, 0, mapping, OPUS_UNIMPLEMENTED, ret_err);
+ if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+ MSenc = opus_multistream_encoder_create(8000, 0, 1, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+ if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+ MSenc = opus_multistream_encoder_create(44100, 2, 2, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+ if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+ MSenc = opus_multistream_encoder_create(8000, 2, 2, 3, mapping, OPUS_APPLICATION_VOIP, ret_err);
+ if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+ MSenc = opus_multistream_encoder_create(8000, 2, -1, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+ if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+ MSenc = opus_multistream_encoder_create(8000, 256, 2, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+ if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+ }
+
MSenc = opus_multistream_encoder_create(8000, 2, 2, 0, mapping, OPUS_APPLICATION_AUDIO, &err);
if(err != OPUS_OK || MSenc==NULL)test_failed();
--
GitLab