sippy / rtpproxy

The RTPproxy is a high-performance software proxy for RTP streams that can work together with Sippy B2BUA, Kamailio, OpenSIPS and SER.
http://rtpproxy.org
BSD 2-Clause "Simplified" License
404 stars 114 forks source link

Segmentation fault in case of RTPP_PTU_ONEMORE return #114

Closed CyBHFal closed 3 years ago

CyBHFal commented 3 years ago

In that case, fds[] pointers are NULL so it will crash if we continue

CyBHFal commented 3 years ago

I didn't compare with RTPP_PTU_OK (instead of 0) because it is not available

sobomax commented 3 years ago

Hi, thanks for submission! I however fail to understand the exact code path that leads to this crash. In particular, rtpp_create_listener() function does not call create_twinlistener() directly, instead the wrapper method get_port (rtpp_ptbl_get_port) is invoked and looking at its code the only two possible values being returned are 0 and -1.

Please advise, some instructions to reproduce would be really helpful!

static int
rtpp_ptbl_get_port(struct rtpp_port_table *self, rtpp_pt_use_t use_port, void *uarg)
{
    struct rtpp_ptbl_priv *pvt;
    int i, j, idx, rval;
    uint16_t port;

    PUB2PVT(self, pvt);

    pthread_mutex_lock(&pvt->lock);
    for (i = 1; i < pvt->port_table_len; i++) {
        idx = (pvt->port_table_idx + i) % pvt->port_table_len;
        port = pvt->port_table[idx];
        if (port == pvt->port_ctl || port == (pvt->port_ctl - 1))
            continue;
        rval = use_port(port, uarg);
        if (!pvt->seq_ports) {
            /* Shuffle table as we go, so we are not easy to outguess */
            j = random() % pvt->port_table_len;
            pvt->port_table[idx] = pvt->port_table[j];
            pvt->port_table[j] = port;
        }
        if (rval == RTPP_PTU_OK) {
            pvt->port_table_idx = idx;
            pthread_mutex_unlock(&pvt->lock);
            return 0;
        }
        if (rval != RTPP_PTU_ONEMORE) {
            pvt->port_table_idx = idx;
            break;
        }
    }
    pthread_mutex_unlock(&pvt->lock);
    return -1;
}
CyBHFal commented 3 years ago

Hi, don't thanks me, it's a mistake in a change i had to do for our need. Very bad to have miss that and lost your time. So sorry.

sobomax commented 3 years ago

False positive. No worries, happens to the best of us! Thanks for being active.