[MPlayer-dev-eng] [PATCH] Use unrar for open vobsubs if available

[Rich Felker]

On Sat, Nov 24, 2007 at 02:50:59PM +0800, Ulion wrote:
> +#include <sys/stat.h>
> +#include "mp_msg.h"
> +#ifndef _WIN32
> +#include "config.h"
> +#define BIN_UNRAR_NAME "./unrar"
> +#ifdef MACOSX
> +#include "get_path.h"
> +#endif /* MACOSX */
> +#define PATHDELIMSTR "/"
> +#else /* _WIN32 */
> +#define BIN_UNRAR_NAME ".\\unrar.exe"
> +#define PATHDELIMSTR "\\"
> +#endif /* _WIN32 */

WTF, this is all utter nonsense. Path delimiter is the same on all
systems and it is /. Moreover, looking for unrar in . is a major vuln.
The PATH exists for a reason; use it.

> +    if (stat(cmd, &st) || !(st.st_mode&S_IXOTH)) {
> +#ifdef MACOSX_BUNDLE
> +        cmd = get_path(BIN_UNRAR_NAME);
> +        if (stat(cmd, &st) || !(st.st_mode&S_IXOTH)) {
> +#endif

Looking in MPlayer config dirs is just bogus, too.

> +    if (libpassword && strlen(libpassword))
> +        sprintf(pwd, "'-p%s'", libpassword);
> +    else
> +        pwd[0] = '\0';
> +
> +    sprintf(cmdline, "'%s' p -inul %s '%s' '%s'", cmd, pwd, rarfile, filename);

Vulnerable overflows and much worse. Consider for example if filename
or rarfile happened to be:
....' & rm -rf ~ & '....

> +    fp = popen(cmdline, "r");

popen is almost always a vuln if any part of the command line comes
from untrusted sources. You must either perform full shell quoting of
all arguments, or else use fork() and exec() manually.

Rich