VLC SMB URI Remote Stack Buffer Overflow
This vulnerability was disclosed on 24 June 2009 and affects VLC player up to 0.9.9a release (latest by now). Here is the vulnerable code as seen in modules/access/smb.c.
#ifdef WIN32
static void Win32AddConnection( access_t *p_access, char *psz_path,
char *psz_user, char *psz_pwd,
char *psz_domain )
{
DWORD (*OurWNetAddConnection2)( LPNETRESOURCE, LPCTSTR, LPCTSTR, DWORD );
char psz_remote[MAX_PATH], psz_server[MAX_PATH], psz_share[MAX_PATH];
...
sprintf( psz_remote, "\\\\%s\\%s", psz_server, psz_share );
...
FreeLibrary( hdll );
}
#endif // WIN32
As you can easily realize, this affects only Windows platform and it is a classic sprintf overflow in psz_remote which has size of MAX_PATH. An attacker can trick the victim into opening a malicous SMB share using VLC to execute arbitrary code. The patch that fixes this bug is:
}
- sprintf( psz_remote, "\\\\%s\\%s", psz_server, psz_share );
+ snprintf( psz_remote, sizeof( psz_remote ), "\\\\%s\\%s", psz_server, psz_share );
net_resource.lpRemoteName = psz_remote;

Now imagine that “\\\\%s\\%s” will result in a string exactly MAX_PATH chars long, just from looking at the patch presented here, I’d say we have some memory disclosure at least (after the patch).
oxff said this on July 4, 2009 at 12:14 pm
And there is still a truncation there, if the first string is equal to MAX_PATH, then the second one will be truncated.
xorl said this on July 6, 2009 at 4:08 pm
While the memory disclosure is true for programs built with visual studio (_snprintf from msvcrt doesn’t guarantee the null termination), that is not a problem here because vlc is built with mingw, and mingw provides its own implementation of snprintf, which is safer.
And true, there is still a truncation.
geal said this on September 5, 2009 at 7:44 am