Opus crash [@opus_decoder_get_nb_samples]

VERIFIED FIXED

Status

()

defect
--
critical
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: posidron, Assigned: rillian)

Tracking

({crash, testcase})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Posted file callstack
Looks like a Opus decoder core bug (null dereference crash)


dec is 0x0


int opus_decoder_get_nb_samples(const OpusDecoder *dec,
      const unsigned char packet[], int len)
{
   int samples;
   int count = opus_packet_get_nb_frames(packet, len);

   if (count<0)
      return count;

   samples = count*opus_packet_get_samples_per_frame(packet, dec->Fs); // <--
   /* Can't have more than 120 ms */
   if (samples*25 > dec->Fs*3)
      return OPUS_INVALID_PACKET;
   else
      return samples;
}
Posted file testcase
The author's tell me it is intentional that opus_decoder_get_nb_samples() and friends do not check argument validity, to make it more obvious when the caller isn't being careful.

So I need to find where mDecoder isn't being initialized.
Assignee: nobody → giles
I was failing to guard the ReconstructGranulepos in nsOpusState::PageIn with mDoneReadingHeaders. This ensures the decoder context is available when we ask it to calculate packet lengths.

I didn't realize PageIn was called before the headers are parsed, but nsOggReader::ReadMetadata does so. The other nsOggCodecState classes already guard this way.

Helpful report, thank you!
Blocks: fuzzing-opus
Rolled into opus-decode10.patch in bug 674225, prior to that patch landing on m-c.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.