FPE on division by zero

[nbars]

Describe the bug

In the following line

pjproject/pjsip-apps/src/samples/siprtp.c

Line 1267 in 08578e8

msec_interval = strm->samples_per_frame * 1000 / strm->clock_rate;

a SIGFPE might me raised because of strm->clock_rate being zero.

Steps to reproduce

None

PJSIP version

2.14

Context

None

Log, call stack, etc

==3299801==ERROR: AddressSanitizer: FPE on unknown address 0x5555556f5e56 (pc 0x5555556f5e56 bp 0x7ffff49fcb90 sp 0x7ffff49fc000 T3)
    #0 0x5555556f5e56 in media_thread pjsip-apps/build/../src/samples/siprtp.c:1267

[sauwming]

strm->clock_rate cannot be zero because it's just filled with the information from the sdp:
audio->clock_rate = audio->si.fmt.clock_rate;

If I tested it here by creating an offer that contains a codec with zero clock rate, such as:
a=rtpmap:120 speex/000, the SDP nego will fail:
siprtp.c ..SDP negotiation failed: No suitable codec for remote offer (PJMEDIA_SDPNEG_NOANSCODEC)

So I wonder how you managed to trigger the issue. Please provide us the incoming SDP offer so we can check.

[nbars]

Hey, I managed to reproduce the bug and can provide you with a pernosco session. It seems like there was no codec negotiated at all:

Thread 4 hit Breakpoint 1, call_on_media_update (inv=0x515000010480, status=0) at ../src/samples/siprtp.c:1463
1463	    audio->clock_rate = audio->si.fmt.clock_rate;
(rr) print audio->si.fmt
$2 = {type = PJMEDIA_TYPE_NONE, pt = 0, encoding_name = {ptr = 0x0, slen = 0}, clock_rate = 0, channel_cnt = 0}

but I am not familiar enough with your code base to debug this further.

[sauwming]

Unfortunately, the session stops rather late at assert(stream->clock_rate != 0). I need it to stop at the moment when audio->clock_rate == 0 (i.e. the assertion should be there instead: assert(audio->clock_rate != 0)).

Alternatively, you can enable more detailed logging by calling ./siprtp --app-log=4.

[nbars]

You can spawn a gdb terminal via the toolbox menu. Since this is recorded using rr, you can execute the target in reverse. For example, you may set a breakpoint break somefile:123 and then continue execution in reverse until you hit it via reverse-continue.

[sauwming]

Ah okay, I didn't know pernosco could do that. It's quite a cool feature.

I have created PR #3927 to prevent the issue from happening.

Note that most sample apps in samples folder (such as siprtp) are actually just small, proof-of-concept apps, so it may not include comprehensive checks.
Just an example, siprtp will assume that there is always at least one media and audio will be the first media (i.e. the statement: audio = &call->media[0] right at the beginning of call_on_media_update()). So it's not a suitable app for fuzzing because it will most certainly break.

If you want to do fuzzing, I would recommend to use pjsua sample app, or create your own fuzzing apps similar to the one that can be found in https://github.com/pjsip/pjproject/tree/master/tests/fuzz.

[nbars]

Alright, thanks for the heads up!