summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiika-Petteri Matikainen <miikapetterim@spotify.com>2019-08-09 13:02:52 +0200
committerMiika-Petteri Matikainen <miikapetterim@spotify.com>2019-08-22 12:53:50 +0200
commit550bb0a21c559f63f63bf0ad6019cfd6de1ae526 (patch)
tree70851f69840729b3c25f48a1fee5f457aed78e30
parent293fd1c04f9d4489be6d4b2b1ca8698f2f902e8e (diff)
downloadtremor-550bb0a21c559f63f63bf0ad6019cfd6de1ae526.tar.gz
Backport floo0 out-of-bounds write fix from main branch
Backports commit 80661a13c93a01f25b8df4e89fecad0eee69ddcc from tremor main branch: floor0 code could potentially use a book where the number of vals it needed to decode was not an integer number of dims wide. This caused it to overflow the output vector as the termination condition was in the outer loop of vorbis_book_decodev_set. None of the various vorbis_book_decodeXXXX calls internally guard against this case either, but in every other use the calling code does properly guard (and avoids putting more checks in the tight inner decode loop). For floor0, move the checks into the inner loop as there's little penalty for doing so. Add commentary indicating where guarding is done for each call variant.
-rw-r--r--codebook.c13
-rw-r--r--floor0.c5
2 files changed, 11 insertions, 7 deletions
diff --git a/codebook.c b/codebook.c
index 9157228..295a9c1 100644
--- a/codebook.c
+++ b/codebook.c
@@ -712,6 +712,7 @@ int decode_map(codebook *s, oggpack_buffer *b, ogg_int32_t *v, int point){
}
/* returns 0 on OK or -1 on eof *************************************/
+/* decode vector / dim granularity guarding is done in the upper layer */
long vorbis_book_decodevs_add(codebook *book,ogg_int32_t *a,
oggpack_buffer *b,int n,int point){
if(book->used_entries>0){
@@ -728,6 +729,7 @@ long vorbis_book_decodevs_add(codebook *book,ogg_int32_t *a,
return 0;
}
+/* decode vector / dim granularity guarding is done in the upper layer */
long vorbis_book_decodev_add(codebook *book,ogg_int32_t *a,
oggpack_buffer *b,int n,int point){
if(book->used_entries>0){
@@ -743,6 +745,9 @@ long vorbis_book_decodev_add(codebook *book,ogg_int32_t *a,
return 0;
}
+/* unlike the others, we guard against n not being an integer number
+ * of <dim> internally rather than in the upper layer (called only by
+ * floor0) */
long vorbis_book_decodev_set(codebook *book,ogg_int32_t *a,
oggpack_buffer *b,int n,int point){
if(book->used_entries>0){
@@ -751,21 +756,21 @@ long vorbis_book_decodev_set(codebook *book,ogg_int32_t *a,
for(i=0;i<n;){
if(decode_map(book,b,v,point))return -1;
- for (j=0;j<book->dim;j++)
+ for (j=0;i<n && j<book->dim;j++)
a[i++]=v[j];
}
}else{
- int i,j;
+ int i;
for(i=0;i<n;){
- for (j=0;j<book->dim;j++)
- a[i++]=0;
+ a[i++]=0;
}
}
return 0;
}
+/* decode vector / dim granularity guarding is done in the upper layer */
long vorbis_book_decodevv_add(codebook *book,ogg_int32_t **a,
long offset,int ch,
oggpack_buffer *b,int n,int point){
diff --git a/floor0.c b/floor0.c
index 864af70..46510bf 100644
--- a/floor0.c
+++ b/floor0.c
@@ -393,10 +393,9 @@ ogg_int32_t *floor0_inverse1(vorbis_dsp_state *vd,vorbis_info_floor *i,
codebook *b=ci->book_param+info->books[booknum];
ogg_int32_t last=0;
- for(j=0;j<info->order;j+=b->dim)
- if(vorbis_book_decodev_set(b,lsp+j,&vd->opb,b->dim,-24)==-1)goto eop;
+ if(vorbis_book_decodev_set(b,lsp,&vd->opb,info->order,-24)==-1)goto eop;
for(j=0;j<info->order;){
- for(k=0;k<b->dim;k++,j++)lsp[j]+=last;
+ for(k=0;j<info->order && k<b->dim;k++,j++)lsp[j]+=last;
last=lsp[j-1];
}