[libav-devel] [PATCH 2/2] rtmp: check malloc calls

Martin Storsjö martin at martin.st
Tue May 22 22:48:38 CEST 2012


On Tue, 22 May 2012, Samuel Pitoiset wrote:

> ---
> libavformat/rtmpproto.c |   73 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 59 insertions(+), 14 deletions(-)
>
> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
> index 4c46a1d..250a3f3 100644
> --- a/libavformat/rtmpproto.c
> +++ b/libavformat/rtmpproto.c
> @@ -497,7 +497,7 @@ static int gen_bytes_read(URLContext *s, RTMPContext *rt, uint32_t ts)
>  * @param keylen digest key length
>  * @param dst    buffer where calculated digest will be stored (32 bytes)
>  */
> -static void rtmp_calc_digest(const uint8_t *src, int len, int gap,
> +static int rtmp_calc_digest(const uint8_t *src, int len, int gap,
>                              const uint8_t *key, int keylen, uint8_t *dst)
> {
>     struct AVSHA *sha;
> @@ -505,6 +505,8 @@ static void rtmp_calc_digest(const uint8_t *src, int len, int gap,
>     int i;
>
>     sha = av_mallocz(av_sha_size);
> +    if (!sha)
> +        return AVERROR(ENOMEM);
>
>     if (keylen < 64) {
>         memcpy(hmac_buf, key, keylen);
> @@ -533,6 +535,8 @@ static void rtmp_calc_digest(const uint8_t *src, int len, int gap,
>     av_sha_final(sha, dst);
>
>     av_free(sha);
> +
> +    return 0;
> }
>
> /**
> @@ -545,14 +549,18 @@ static void rtmp_calc_digest(const uint8_t *src, int len, int gap,
> static int rtmp_handshake_imprint_with_digest(uint8_t *buf)
> {
>     int i, digest_pos = 0;
> +    int ret;
>
>     for (i = 8; i < 12; i++)
>         digest_pos += buf[i];
>     digest_pos = (digest_pos % 728) + 12;
>
> -    rtmp_calc_digest(buf, RTMP_HANDSHAKE_PACKET_SIZE, digest_pos,
> +    ret = rtmp_calc_digest(buf, RTMP_HANDSHAKE_PACKET_SIZE, digest_pos,
>                      rtmp_player_key, PLAYER_KEY_OPEN_PART_LEN,
>                      buf + digest_pos);
> +    if (ret < 0)
> +        return ret;
> +
>     return digest_pos;
> }
>
> @@ -567,14 +575,18 @@ static int rtmp_validate_digest(uint8_t *buf, int off)
> {
>     int i, digest_pos = 0;
>     uint8_t digest[32];
> +    int ret;
>
>     for (i = 0; i < 4; i++)
>         digest_pos += buf[i + off];
>     digest_pos = (digest_pos % 728) + off + 4;
>
> -    rtmp_calc_digest(buf, RTMP_HANDSHAKE_PACKET_SIZE, digest_pos,
> +    ret = rtmp_calc_digest(buf, RTMP_HANDSHAKE_PACKET_SIZE, digest_pos,
>                      rtmp_server_key, SERVER_KEY_OPEN_PART_LEN,
>                      digest);
> +    if (ret < 0)
> +        return ret;
> +
>     if (!memcmp(digest, buf + digest_pos, 32))
>         return digest_pos;
>     return 0;
> @@ -602,6 +614,7 @@ static int rtmp_handshake(URLContext *s, RTMPContext *rt)
>     int i;
>     int server_pos, client_pos;
>     uint8_t digest[32];
> +    int ret;
>
>     av_log(s, AV_LOG_DEBUG, "Handshaking...\n");
>
> @@ -610,17 +623,21 @@ static int rtmp_handshake(URLContext *s, RTMPContext *rt)
>     for (i = 9; i <= RTMP_HANDSHAKE_PACKET_SIZE; i++)
>         tosend[i] = av_lfg_get(&rnd) >> 24;
>     client_pos = rtmp_handshake_imprint_with_digest(tosend + 1);
> +    if (client_pos < 0) {
> +        /* out of memory */
> +        return client_pos;
> +    }
>
>     ffurl_write(rt->stream, tosend, RTMP_HANDSHAKE_PACKET_SIZE + 1);
>     i = ffurl_read_complete(rt->stream, serverdata, RTMP_HANDSHAKE_PACKET_SIZE + 1);
>     if (i != RTMP_HANDSHAKE_PACKET_SIZE + 1) {
>         av_log(s, AV_LOG_ERROR, "Cannot read RTMP handshake response\n");
> -        return -1;
> +        return AVERROR(EIO);
>     }
>     i = ffurl_read_complete(rt->stream, clientdata, RTMP_HANDSHAKE_PACKET_SIZE);
>     if (i != RTMP_HANDSHAKE_PACKET_SIZE) {
>         av_log(s, AV_LOG_ERROR, "Cannot read RTMP handshake response\n");
> -        return -1;
> +        return AVERROR(EIO);
>     }
>
>     av_log(s, AV_LOG_DEBUG, "Server version %d.%d.%d.%d\n",
> @@ -628,33 +645,54 @@ static int rtmp_handshake(URLContext *s, RTMPContext *rt)
>
>     if (rt->is_input && serverdata[5] >= 3) {
>         server_pos = rtmp_validate_digest(serverdata + 1, 772);
> +        if (server_pos < 0) {
> +            /* out of memory */
> +            return server_pos;
> +        }
> +
>         if (!server_pos) {
>             server_pos = rtmp_validate_digest(serverdata + 1, 8);
> +            if (server_pos < 0) {
> +                /* out of memory */
> +                return server_pos;
> +            }
> +

This same pattern is repeated quite a few times, and is quite verbose (it 
makes the error handling more prominent than the actual application 
logic). I'd skip the braces and the comment, which gets it down to 2 lines 
instead of 4.

// Martin


More information about the libav-devel mailing list