[MPlayer-dev-eng] [PATCH] loader/win32.c cleanup of CreateFile should fix vp6vfw 2nd pass encoding on linux

Gianluigi Tiesi mplayer at netfarm.it
Tue Oct 9 02:23:34 CEST 2007


Hello,

I've made an updated patch, I've added a check
that CreateFileA would fail if the file exists and it's
not a regular file, i.e. a symlink, since
this can be a security issue

Regards

-- 
Gianluigi Tiesi <sherpya at netfarm.it>
EDP Project Leader
Netfarm S.r.l. - http://www.netfarm.it/
Free Software: http://oss.netfarm.it/
-------------- next part --------------
diff -NuBr -x.svn -xvidix -xhelp_mp.h -xlibdha -x'*.so' -x'*.log' -x'*.a' -x'*.exe' -x'*.o' -xconfigure.log -xconfig.mak -x.cvsignore -xconfig.h -xcodecs.conf.h -xversion.h -x.depend main/loader/win32.c sherpya/loader/win32.c
--- main/loader/win32.c	2007-09-13 20:43:22.671875000 +0200
+++ sherpya/loader/win32.c	2007-10-09 02:18:20.031250000 +0200
@@ -60,6 +60,7 @@
 #include <math.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <dirent.h>
 #include <sys/time.h>
@@ -3486,102 +3487,82 @@
 // They try to open APmpeg4v1.apl with it.
 // DLL will close opened file with CloseHandle().
 //
-static HANDLE WINAPI expCreateFileA(LPCSTR cs1,DWORD i1,DWORD i2,
-				    LPSECURITY_ATTRIBUTES p1, DWORD i3,DWORD i4,HANDLE i5)
+// refer to http://msdn2.microsoft.com/en-us/library/aa363858.aspx
+// for CreateFile flags
+static HANDLE WINAPI expCreateFileA(LPCSTR lpFileName, DWORD dwDesiredAccess,
+                                    DWORD dwShareMode, LPSECURITY_ATTRIBUTES lpSecurityAttributes,
+                                    DWORD dwCreationDisposition, DWORD dwFlagsAndAttributes,
+                                    HANDLE hTemplateFile)
 {
-    dbgprintf("CreateFileA(0x%x='%s', %d, %d, 0x%x, %d, %d, 0x%x)\n", cs1, cs1, i1,
-	      i2, p1, i3, i4, i5);
-    if((!cs1) || (strlen(cs1)<2))return -1;
+    char dest[MAX_PATH + 1] = "/tmp/";
+    char *p = dest + 5;
+    int flags = 0;
+    struct stat info;
 
-#ifdef QTX
-    if(strstr(cs1, "QuickTime.qts"))
-    {
-	int result;
-	char* tmp=malloc(strlen(def_path)+50);
-	strcpy(tmp, def_path);
-	strcat(tmp, "/");
-	strcat(tmp, "QuickTime.qts");
-	result=open(tmp, O_RDONLY);
-	free(tmp);
-	return result;
-    }
-    if(strstr(cs1, ".qtx"))
-    {
-	int result;
-	char* tmp=malloc(strlen(def_path)+250);
-	char* x=strrchr(cs1,'\\');
-	sprintf(tmp,"%s/%s",def_path,x?(x+1):cs1);
-//	printf("### Open: %s -> %s\n",cs1,tmp);
-	result=open(tmp, O_RDONLY);
-	free(tmp);
-	return result;
-    }
-#endif
+    dbgprintf("CreateFileA(): Filename %s - dwDesiredAccess 0x%08x - dwCreationDisposition 0x%08x\n",
+              lpFileName, dwDesiredAccess, dwCreationDisposition);
 
-    if(strncmp(cs1, "AP", 2) == 0)
-    {
-	int result;
-	char* tmp=malloc(strlen(def_path)+50);
-	strcpy(tmp, def_path);
-	strcat(tmp, "/");
-	strcat(tmp, "APmpg4v1.apl");
-	result=open(tmp, O_RDONLY);
-	free(tmp);
-	return result;
-    }
-    if (strstr(cs1, "vp3"))
-    {
-	int r;
-	int flg = 0;
-	char* tmp=malloc(20 + strlen(cs1));
-	strcpy(tmp, "/tmp/");
-	strcat(tmp, cs1);
-	r = 4;
-	while (tmp[r])
-	{
-	    if (tmp[r] == ':' || tmp[r] == '\\')
-		tmp[r] = '_';
-	    r++;
-	}
-	if (GENERIC_READ & i1)
-	    flg |= O_RDONLY;
-	else if (GENERIC_WRITE & i1)
-	{
-	    flg |= O_WRONLY;
-	    printf("Warning: openning filename %s  %d (flags; 0x%x) for write\n", tmp, r, flg);
-	}
-	r=open(tmp, flg);
-	free(tmp);
-	return r;
-    }
+    /* lpFileName is NULL */
+    if (!lpFileName)
+        return INVALID_HANDLE_VALUE;
+
+    strncat(dest, lpFileName, MAX_PATH);
+    dest[MAX_PATH] = 0;
 
-    // Needed by wnvplay1.dll
-    if (strstr(cs1, "WINNOV.bmp"))
+    /* avoid directory traversal */
+    if (strstr(dest, ".."))
     {
-	int r;
-	r=open("/dev/null", 0);
-	return r;
+        dbgprintf("CreateFileA(): Filename %s - possible directory traversal\n", dest);
+        return INVALID_HANDLE_VALUE;
     }
 
-#if 0
-    /* we need this for some virtualdub filters */
+    /* Sanitize the filename */
+    while (*p)
     {
-	int r;
-	int flg = 0;
-	if (GENERIC_READ & i1)
-	    flg |= O_RDONLY;
-	else if (GENERIC_WRITE & i1)
-	{
-	    flg |= O_WRONLY;
-	    printf("Warning: openning filename %s  %d (flags; 0x%x) for write\n", cs1, r, flg);
-	}
-	r=open(cs1, flg);
-	return r;
+        if ((*p == '/') || (*p == '\\') || (*p == ':'))
+            *p = '_';
+        p++;
     }
-#endif
 
-    return atoi(cs1+2);
+    /* CreateFile on a directory has no unix equivalent,
+     * non regular files (i.e. symlinks) can be a potential
+     * security issue, so we only accept regular files
+     */
+    if (lstat(dest, &info) == 0)
+        if (!S_ISREG(info.st_mode))
+        {
+            dbgprintf("CreateFileA(): Filename %s - is not a regular file\n", dest);
+            return INVALID_HANDLE_VALUE;
+        }
+
+    /* Desidered Access */
+    if ((dwDesiredAccess & GENERIC_READ) &&
+        (dwDesiredAccess & GENERIC_WRITE))
+        flags |= O_RDWR;
+
+    if (dwDesiredAccess & GENERIC_READ) flags |= O_RDONLY;
+    if (dwDesiredAccess & GENERIC_WRITE) flags |= O_WRONLY;
+
+    /* Creation Disposition */
+    switch (dwCreationDisposition)
+    {
+        case CREATE_ALWAYS:
+            flags |= O_CREAT | O_TRUNC;
+            break;
+        case CREATE_NEW:
+            flags |= O_CREAT | O_EXCL;
+            break;
+        case TRUNCATE_EXISTING:
+            flags |= O_TRUNC;
+            break;
+        case OPEN_ALWAYS: /* flags ? */
+        case OPEN_EXISTING: /* flags ? */
+        default:
+            break;
+    }
+    return open(dest, flags);
 }
+
 static UINT WINAPI expGetSystemDirectoryA(
   char* lpBuffer,  // address of buffer for system directory
   UINT uSize        // size of directory buffer
@@ -3660,7 +3641,7 @@
 static DWORD  WINAPI expSetFilePointer(HANDLE h, LONG val, LPLONG ext, DWORD whence)
 {
     int wh;
-    dbgprintf("SetFilePointer(%d, 0x%x, 0x%x = %d, %d)\n", h, val, ext, *ext, whence);
+    dbgprintf("SetFilePointer(%d, 0x%x, 0x%x = %d, %d)\n", h, val, ext, ext ? *ext : NULL, whence);
     //why would DLL want temporary file with >2Gb size?
     switch(whence)
     {


More information about the MPlayer-dev-eng mailing list