From 50fd339b4302710eee9a2e06c393dd9ee12c7566 Mon Sep 17 00:00:00 2001
From: "Timothy B. Terriberry" <tterribe@xiph.org>
Date: Tue, 26 Jul 2016 18:29:39 -0700
Subject: [PATCH] Sanity improvements to oc_dec_headerin() error returns.
In the case where we got a data packet before receiving all three
headers, the old code would check to see if the second through
seventh bytes matched the magic string "theora" (extremely
unlikely, but possible), and if so, return TH_EBADHEADER, otherwise
return TH_ENOTFORMAT.
That this was not consistent was a bit non-sensical.
5a5f5bb20c74 changed to returning TH_EBADHEADER if we got a data
packet after receiving the first header, but left the old behavior
for the first packet.
Change instead to explicitly return TH_ENOTFORMAT if the first
header was missing (since we only check one bit of the packet to
determine whether or not it's a data packet, odds are it's a packet
for some other kind of data, like a Vorbis header).
We continue to return TH_EBADHEADER if we see a data packet after
encountering a valid header, but before reading all three.
Also re-arrange the NULL checks to follow continue to allow the
undocumented ability to pass in NULL for parameters which are not
needed by the next header in the sequence.
E.g., it's perfectly all right to pass NULL for _setup when
expecting to read the comment header next.
In this case we'll now return TH_EBADHEADER instead of TH_EFAULT if
the packet was actually a data packet.
---
lib/decinfo.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/lib/decinfo.c b/lib/decinfo.c
index 4e21e648..80e3f03e 100644
--- a/lib/decinfo.c
+++ b/lib/decinfo.c
@@ -172,16 +172,23 @@ static int oc_dec_headerin(oc_pack_buf *_opb,th_info *_info,
int ret;
val=oc_pack_read(_opb,8);
packtype=(int)val;
- /*If we're at a data packet and we have received all three headers, we're
- done.*/
- if(!(packtype&0x80)&&_info->frame_width>0){
- /*Our documentation says we'll return TH_EFAULT if these are NULL, so let's
- check.*/
- if(_tc==NULL||_setup==NULL)return TH_EFAULT;
- /*Otherwise, if we didn't get all of the headers, this was not the next
- packet in the expected sequence.*/
- else if(_tc->vendor==NULL||*_setup==NULL)return TH_EBADHEADER;
- /*Otherwise, we're done.*/
+ /*If we're at a data packet...*/
+ if(!(packtype&0x80)){
+ /*Check to make sure we received all three headers...
+ If we haven't seen any valid headers, assume this is not actually
+ Theora.*/
+ if(_info->frame_width<=0)return TH_ENOTFORMAT;
+ /*Follow our documentation, which says we'll return TH_EFAULT if this
+ are NULL (_info was checked by our caller).*/
+ if(_tc==NULL)return TH_EFAULT;
+ /*And if any other headers were missing, declare this packet "out of
+ sequence" instead.*/
+ if(_tc->vendor==NULL)return TH_EBADHEADER;
+ /*Don't check this until it's needed, since we allow passing NULL for the
+ arguments that we're not expecting the next header to fill in yet.*/
+ if(_setup==NULL)return TH_EFAULT;
+ if(*_setup==NULL)return TH_EBADHEADER;
+ /*If we got everything, we're done.*/
return 0;
}
/*Check the codec string.*/
--
GitLab