The default bug view has changed. See this FAQ.

Opus crash [@opus_decoder_get_nb_samples]

VERIFIED FIXED

Status

()

Core
Audio/Video
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: posidron, Assigned: rillian)

Tracking

(Blocks: 1 bug, {crash, testcase})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 616465 [details]
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;
}
(Reporter)

Comment 1

5 years ago
Created attachment 616466 [details]
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
Created attachment 619092 [details] [diff] [review]
guard ReconstructGranulepos in nsOpusState::PageIn

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!
(Reporter)

Updated

5 years ago
Blocks: 750714
Rolled into opus-decode10.patch in bug 674225, prior to that patch landing on m-c.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.