[xiph-commits] r17268 - trunk/ogg/src
xiphmont at svn.xiph.org
xiphmont at svn.xiph.org
Thu Jun 3 22:19:12 PDT 2010
Author: xiphmont
Date: 2010-06-03 22:19:12 -0700 (Thu, 03 Jun 2010)
New Revision: 17268
Modified:
trunk/ogg/src/bitwise.c
Log:
Complete code review on the pattern:
if(b->endbyte*8+bits>b->storage*8)goto overflow;
Eliminate the possibility of b->endbyte overflow on buffer storage near or
exactly at long storage limit; corrections made to both read and write.
Also, harden both read and write against requesting <0 or >32 read/write. In both
case, the packer is put into 'error' state.
Modified: trunk/ogg/src/bitwise.c
===================================================================
--- trunk/ogg/src/bitwise.c 2010-06-02 01:42:37 UTC (rev 17267)
+++ trunk/ogg/src/bitwise.c 2010-06-04 05:19:12 UTC (rev 17268)
@@ -80,14 +80,13 @@
/* Takes only up to 32 bits. */
void oggpack_write(oggpack_buffer *b,unsigned long value,int bits){
- if(b->endbyte+4>=b->storage){
+ if(bits<0 || bits>32) goto err;
+ if(b->endbyte>=b->storage-4){
void *ret;
if(!b->ptr)return;
+ if(b->storage+BUFFER_INCREMENT<b->storage) goto err;
ret=_ogg_realloc(b->buffer,b->storage+BUFFER_INCREMENT);
- if(!ret){
- oggpack_writeclear(b);
- return;
- }
+ if(!ret) goto err;
b->buffer=ret;
b->storage+=BUFFER_INCREMENT;
b->ptr=b->buffer+b->endbyte;
@@ -117,18 +116,20 @@
b->endbyte+=bits/8;
b->ptr+=bits/8;
b->endbit=bits&7;
+ return;
+ err:
+ oggpack_writeclear(b);
}
/* Takes only up to 32 bits. */
void oggpackB_write(oggpack_buffer *b,unsigned long value,int bits){
- if(b->endbyte+4>=b->storage){
+ if(bits<0 || bits>32) goto err;
+ if(b->endbyte>=b->storage-4){
void *ret;
if(!b->ptr)return;
+ if(b->storage+BUFFER_INCREMENT<b->storage) goto err;
ret=_ogg_realloc(b->buffer,b->storage+BUFFER_INCREMENT);
- if(!ret){
- oggpack_writeclear(b);
- return;
- }
+ if(!ret) goto err;
b->buffer=ret;
b->storage+=BUFFER_INCREMENT;
b->ptr=b->buffer+b->endbyte;
@@ -158,6 +159,9 @@
b->endbyte+=bits/8;
b->ptr+=bits/8;
b->endbit=bits&7;
+ return;
+ err:
+ oggpack_writeclear(b);
}
void oggpack_writealign(oggpack_buffer *b){
@@ -193,13 +197,11 @@
/* aligned block copy */
if(b->endbyte+bytes+1>=b->storage){
void *ret;
- if(!b->ptr)return;
+ if(!b->ptr) goto err;
+ if(b->storage=b->endbyte+bytes+BUFFER_INCREMENT>b->storage) goto err;
b->storage=b->endbyte+bytes+BUFFER_INCREMENT;
ret=_ogg_realloc(b->buffer,b->storage);
- if(!ret){
- oggpack_writeclear(b);
- return;
- }
+ if(!ret) goto err;
b->buffer=ret;
b->ptr=b->buffer+b->endbyte;
}
@@ -216,6 +218,9 @@
else
w(b,(unsigned long)(ptr[bytes]),bits);
}
+ return;
+ err:
+ oggpack_writeclear(b);
}
void oggpack_writecopy(oggpack_buffer *b,void *source,long bits){
@@ -259,15 +264,20 @@
/* Read in bits without advancing the bitptr; bits <= 32 */
long oggpack_look(oggpack_buffer *b,int bits){
unsigned long ret;
- unsigned long m=mask[bits];
+ unsigned long m;
+ if(bits<0 || bits>32) return -1;
+ m=mask[bits];
bits+=b->endbit;
- if(b->endbyte+4>=b->storage){
+ if(b->endbyte >= b->storage-4){
/* not the main path */
- if(b->endbyte*8+bits>b->storage*8)return(-1);
+ if(b->endbyte > b->storage-((bits+7)>>3)) return -1;
+ /* special case to avoid reading b->ptr[0], which might be past the end of
+ the buffer; also skips some useless accounting */
+ else if(!bits)return(0L);
}
-
+
ret=b->ptr[0]>>b->endbit;
if(bits>8){
ret|=b->ptr[1]<<(8-b->endbit);
@@ -288,13 +298,17 @@
unsigned long ret;
int m=32-bits;
+ if(m<0 || m>32) return -1;
bits+=b->endbit;
- if(b->endbyte+4>=b->storage){
+ if(b->endbyte >= b->storage-4){
/* not the main path */
- if(b->endbyte*8+bits>b->storage*8)return(-1);
+ if(b->endbyte > b->storage-((bits+7)>>3)) return -1;
+ /* special case to avoid reading b->ptr[0], which might be past the end of
+ the buffer; also skips some useless accounting */
+ else if(!bits)return(0L);
}
-
+
ret=b->ptr[0]<<(24+b->endbit);
if(bits>8){
ret|=b->ptr[1]<<(16+b->endbit);
@@ -322,9 +336,18 @@
void oggpack_adv(oggpack_buffer *b,int bits){
bits+=b->endbit;
+
+ if(b->endbyte > b->storage-((bits+7)>>3)) goto overflow;
+
b->ptr+=bits/8;
b->endbyte+=bits/8;
b->endbit=bits&7;
+ return;
+
+ overflow:
+ b->ptr=NULL;
+ b->endbyte=b->storage;
+ b->endbit=1;
}
void oggpackB_adv(oggpack_buffer *b,int bits){
@@ -346,16 +369,20 @@
/* bits <= 32 */
long oggpack_read(oggpack_buffer *b,int bits){
long ret;
- unsigned long m=mask[bits];
+ unsigned long m;
+ if(bits<0 || bits>32) goto err;
+ m=mask[bits];
bits+=b->endbit;
- if(b->endbyte+4>=b->storage){
+ if(b->endbyte >= b->storage-4){
/* not the main path */
- ret=-1L;
- if(b->endbyte*8+bits>b->storage*8)goto overflow;
+ if(b->endbyte > b->storage-((bits+7)>>3)) goto overflow;
+ /* special case to avoid reading b->ptr[0], which might be past the end of
+ the buffer; also skips some useless accounting */
+ else if(!bits)return(0L);
}
-
+
ret=b->ptr[0]>>b->endbit;
if(bits>8){
ret|=b->ptr[1]<<(8-b->endbit);
@@ -370,31 +397,35 @@
}
}
ret&=m;
-
- overflow:
-
b->ptr+=bits/8;
b->endbyte+=bits/8;
b->endbit=bits&7;
- return(ret);
+ return ret;
+
+ overflow:
+ err:
+ b->ptr=NULL;
+ b->endbyte=b->storage;
+ b->endbit=1;
+ return -1L;
}
/* bits <= 32 */
long oggpackB_read(oggpack_buffer *b,int bits){
long ret;
long m=32-bits;
-
+
+ if(m<0 || m>32) goto err;
bits+=b->endbit;
if(b->endbyte+4>=b->storage){
/* not the main path */
- ret=-1L;
- if(b->endbyte*8+bits>b->storage*8)goto overflow;
+ if(b->endbyte > b->storage-((bits+7)>>3)) goto overflow;
/* special case to avoid reading b->ptr[0], which might be past the end of
the buffer; also skips some useless accounting */
else if(!bits)return(0L);
}
-
+
ret=b->ptr[0]<<(24+b->endbit);
if(bits>8){
ret|=b->ptr[1]<<(16+b->endbit);
@@ -408,27 +439,25 @@
}
}
ret=((ret&0xffffffffUL)>>(m>>1))>>((m+1)>>1);
-
- overflow:
b->ptr+=bits/8;
b->endbyte+=bits/8;
b->endbit=bits&7;
- return(ret);
+ return ret;
+
+ overflow:
+ err:
+ b->ptr=NULL;
+ b->endbyte=b->storage;
+ b->endbit=1;
+ return -1L;
}
long oggpack_read1(oggpack_buffer *b){
long ret;
-
- if(b->endbyte>=b->storage){
- /* not the main path */
- ret=-1L;
- goto overflow;
- }
+ if(b->endbyte >= b->storage) goto overflow;
ret=(b->ptr[0]>>b->endbit)&1;
-
- overflow:
b->endbit++;
if(b->endbit>7){
@@ -436,21 +465,20 @@
b->ptr++;
b->endbyte++;
}
- return(ret);
+ return ret;
+
+ overflow:
+ b->ptr=NULL;
+ b->endbyte=b->storage;
+ b->endbit=1;
+ return -1L;
}
long oggpackB_read1(oggpack_buffer *b){
long ret;
-
- if(b->endbyte>=b->storage){
- /* not the main path */
- ret=-1L;
- goto overflow;
- }
+ if(b->endbyte >= b->storage) goto overflow;
ret=(b->ptr[0]>>(7-b->endbit))&1;
-
- overflow:
b->endbit++;
if(b->endbit>7){
@@ -458,7 +486,13 @@
b->ptr++;
b->endbyte++;
}
- return(ret);
+ return ret;
+
+ overflow:
+ b->ptr=NULL;
+ b->endbyte=b->storage;
+ b->endbit=1;
+ return -1L;
}
long oggpack_bytes(oggpack_buffer *b){
More information about the commits
mailing list