[MPlayer-dev-eng] [PATCH] cleaned up flac metadata parsing

[Reimar Döffinger]

On Wed, Sep 02, 2009 at 11:07:48PM -0600, selfacclaimedgenius at gmail.com wrote:
> -        cn = 0;
> -        for (; cn < comment_list_len; cn++)
> +        for (int i = 0; i < comment_list_len; i++)

That declaration is not compatible with gcc 2.95

> +          // Comment length can't exceed end of block
> +          if (length > blk_len - (unsigned int)ptr) return;

Wtf? That makes no sense. And compiles with a warning.

> +          char c[length];
> +          strncpy(c, comment, length); // Get whole comment
> +          if(length > 0) c[length] = '\0';

And since length can be arbitrary this still is a security hole.

> +          // Separate into key and value
> +          int split_idx = -1;

Also these declarations all give warnings, you should at least look at
the warnings your code generates.
Please, again, if you lack the experience to write secure code _only_
make the modifications necessary to fix the security holes in the
current code or this review will take years and be more effort for me
than fixing the current code myself.