Skip to content

Commit

Permalink
Fix several issues with multistream argument validation.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gmaxwell
gmaxwell committed Oct 27, 2012
1 parent 6930d90 commit de95da9
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 101 deletions.
30 changes: 23 additions & 7 deletions src/opus_multistream.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -665,8 +683,6 @@ OpusMSDecoder *opus_multistream_decoder_create(
st = NULL;
}
return st;


}

typedef void (*opus_copy_channel_out_func)(
Expand Down
209 changes: 115 additions & 94 deletions tests/test_opus_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
23 changes: 23 additions & 0 deletions tests/test_opus_encode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down

0 comments on commit de95da9

Please sign in to comment.