[vlc-devel] [PATCHv4 1/3] access: Add satip access module

[Filip Roséen]

> +static enum rtsp_result rtsp_handle(access_t *access) {
> + [... ]
> +
> +    /* Parse header */
> +    while (!have_header && (in = net_Gets(access, sys->tcp_sock)) != NULL) {
> +        if (strstr(in, "RTSP/1.0 ") != NULL) {
> +            rtsp_result = atoi(in + 9);

The above logic is faulty. I assume the intent is to look for "RTSP/1.0 " and
then treat the following bytes as the textual-representation of an integer,
though this is not what is going on.

If an implementation-specific (or similar) header contains "RTSP/1.0 " you would
overwrite `rtsp_result` with a value with what just happens to be at `[9, ...)`
in `in`, even if a correct value has already been found.

    RTSP/1.0 200
    Content-Base: ...
    Implementation-Specific-Header: RTSP/1.0 korv

The contents of *"Implementation-Specific-Header [...]"* will trigger
`rtsp_result = atoi( in + 9 )`.

> +        } else if (strncmp(in, "Content-Base", 12) == 0) {
> +            free(sys->content_base);
> +            sys->content_base = strdup(in + 14);

Nothing in the above says that `in + 14` is even within the valid range of the
string yield by `net_Gets`; you are really looking for "Content-Base: ", as such
usage of `strncmp( in, "Content-Base: ", 14 )` is far more appropriate.

`strdup( in + 14 )` would then, at most, `strdup` an empty string.

> +        } else if (strncmp(in, "Content-Length", 14) == 0) {
> +            content_length = atoi(in + 16);

See previous remark.

> +        } else if (strncmp("Session:", in, 8) == 0) {
> +            parse_session(in, sys->session_id, 64, &sys->keepalive_interval);
> +        } else if (strncmp("Transport:", in, 10) == 0) {
> +            parse_transport(access, in);
> +        } else if (strncmp("com.ses.streamID:", in, 17) == 0) {
> +            sys->stream_id = atoi(in + 17);
> +        } else if (in[0] == '\0') {
> +            have_header = true;
> +        }

---------------------------------------------------------------------------------

> +/* Bind two adjacent free ports, of which the first one is even (for RTP data)
> + * and the second is odd (RTCP). This is a requirement of the satip
> + * specification */
> +static int satip_bind_ports(access_t *access)
> +{
> +    access_sys_t *sys = access->p_sys;
> +    uint8_t rnd;
> +
> +    vlc_rand_bytes(&rnd, 1);
> +    sys->udp_port = 9000 + (rnd * 2); /* randomly chosen, even start point */

This following loop will, in the worse case, loop forever (given that
`sys->udp_sock` can wrap from `65534` to `0` in each of the two `sys->udp_port
+= 2`).

> +    while (sys->udp_sock < 0 && sys->udp_port < 65535) {
> +        sys->udp_sock = net_OpenDgram(access, "0.0.0.0", sys->udp_port, NULL,
> +                0, IPPROTO_UDP);
> +        if (sys->udp_sock < 0) {
> +            sys->udp_port += 2;
> +            continue;
> +        }
> +
> +        sys->rtcp_sock = net_OpenDgram(access, "0.0.0.0", sys->udp_port + 1, NULL,
> +                0, IPPROTO_UDP);
> +        if (sys->rtcp_sock < 0) {
> +            close(sys->udp_sock);
> +            sys->udp_port += 2;
> +            continue;
> +        }
> +    }
> +
> +    if (sys->udp_sock < 0) {
> +        msg_Err(access, "Could not open two adjacent ports for RTP and RTCP data");
> +        return VLC_EGENERIC;
> +    }
> +
> +    return 0;
> +}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160527/5898e3fa/attachment.html>