[xiph-commits] r17541 - trunk/Tremor

[xiphmont at svn.xiph.org]

Author: xiphmont
Date: 2010-10-18 02:49:20 -0700 (Mon, 18 Oct 2010)
New Revision: 17541

Modified:
   trunk/Tremor/block.c
Log:
Harden the code that trims the last packet of a stream; it was
possible to game the granpos such that the trim code would try to
rewind more samples than were actually available in storage.



Modified: trunk/Tremor/block.c
===================================================================
--- trunk/Tremor/block.c	2010-10-16 21:38:41 UTC (rev 17540)
+++ trunk/Tremor/block.c	2010-10-18 09:49:20 UTC (rev 17541)
@@ -401,17 +401,32 @@
       if(b->sample_count>v->granulepos){
 	/* corner case; if this is both the first and last audio page,
 	   then spec says the end is cut, not beginning */
+	long extra=b->sample_count-vb->granulepos;
+
+        /* we use ogg_int64_t for granule positions because a
+           uint64 isn't universally available.  Unfortunately,
+           that means granposes can be 'negative' and result in
+           extra being negative */
+        if(extra<0)
+          extra=0;
+
 	if(vb->eofflag){
 	  /* trim the end */
 	  /* no preceeding granulepos; assume we started at zero (we'd
 	     have to in a short single-page stream) */
 	  /* granulepos could be -1 due to a seek, but that would result
 	     in a long coun`t, not short count */
-	  
-	  v->pcm_current-=(b->sample_count-v->granulepos);
+
+          /* Guard against corrupt/malicious frames that set EOP and
+             a backdated granpos; don't rewind more samples than we
+             actually have */
+          if(extra > v->pcm_current - v->pcm_returned)
+            extra = v->pcm_current - v->pcm_returned;
+
+	  v->pcm_current-=extra;
 	}else{
 	  /* trim the beginning */
-	  v->pcm_returned+=(b->sample_count-v->granulepos);
+	  v->pcm_returned+=extra;
 	  if(v->pcm_returned>v->pcm_current)
 	    v->pcm_returned=v->pcm_current;
 	}
@@ -429,7 +444,22 @@
 	if(extra)
 	  if(vb->eofflag){
 	    /* partial last frame.  Strip the extra samples off */
-	    v->pcm_current-=extra;
+
+            /* Guard against corrupt/malicious frames that set EOP and
+               a backdated granpos; don't rewind more samples than we
+               actually have */
+            if(extra > v->pcm_current - v->pcm_returned)
+              extra = v->pcm_current - v->pcm_returned;
+
+            /* we use ogg_int64_t for granule positions because a
+               uint64 isn't universally available.  Unfortunately,
+               that means granposes can be 'negative' and result in
+               extra being negative */
+            if(extra<0)
+              extra=0;
+
+            v->pcm_current-=extra;
+
 	  } /* else {Shouldn't happen *unless* the bitstream is out of
 	       spec.  Either way, believe the bitstream } */
       } /* else {Shouldn't happen *unless* the bitstream is out of