diff options
-rw-r--r-- | ChangeLog | 14 | ||||
-rw-r--r-- | win/tclWinFile.c | 70 |
2 files changed, 58 insertions, 26 deletions
@@ -1,7 +1,17 @@ -2012-??-?? Jan Nijtmans <nijtmans@users.sf.net> +2012-02-02 Jan Nijtmans <nijtmans@users.sf.net> * generic/tclUniData.c: [Frq 3464401] Support Unicode 6.1 - * generic/regc_locale.c: (not to be merged until Unicode 6.1 is final!!!) + * generic/regc_locale.c: + +2012-02-02 Don Porter <dgp@users.sourceforge.net> + + * win/tclWinFile.c: [Bugs 2974459,2879351,1951574,1852572, + 1661378,1613456]: Revisions to the NativeAccess() routine that + queries file permissions on Windows native filesystems. Meant to + fix numerous bugs where [file writable|readable|executable] "lies" + about what operations are possible, especially when the file resides + on a Samba share. Patch cherrypicked off the fix-win-native-access + branch. 2012-01-22 Jan Nijtmans <nijtmans@users.sf.net> diff --git a/win/tclWinFile.c b/win/tclWinFile.c index 5a3f193..f145e23 100644 --- a/win/tclWinFile.c +++ b/win/tclWinFile.c @@ -1342,14 +1342,25 @@ NativeAccess( return -1; } - if ((mode & W_OK) - && (tclWinProcs->getFileSecurityProc == NULL) - && (attr & FILE_ATTRIBUTE_READONLY)) { + if (mode == F_OK) { /* - * We don't have the advanced 'getFileSecurityProc', and - * our attributes say the file is not writable. If we - * do have 'getFileSecurityProc', we'll do a more - * robust XP-related check below. + * File exists, nothing else to check. + */ + + return 0; + } + + if ((mode & W_OK) + && (attr & FILE_ATTRIBUTE_READONLY) + && !(attr & FILE_ATTRIBUTE_DIRECTORY)) { + /* + * The attributes say the file is not writable. If the file is a + * regular file (i.e., not a directory), then the file is not + * writable, full stop. For directories, the read-only bit is + * (mostly) ignored by Windows, so we can't ascertain anything about + * directory access from the attrib data. However, if we have the + * advanced 'getFileSecurityProc', then more robust ACL checks + * will be done below. */ Tcl_SetErrno(EACCES); @@ -1373,15 +1384,14 @@ NativeAccess( * we have a more complex permissions structure so we try to check that. * The code below is remarkably complex for such a simple thing as finding * what permissions the OS has set for a file. - * - * If we are simply checking for file existence, then we don't need all - * these complications (which are really quite slow: with this code 'file - * readable' is 5-6 times slower than 'file exists'). */ - if ((mode != F_OK) && (tclWinProcs->getFileSecurityProc != NULL)) { + if (tclWinProcs->getFileSecurityProc != NULL) { SECURITY_DESCRIPTOR *sdPtr = NULL; unsigned long size; + SID *pSid = 0; + BOOL SidDefaulted; + SID_IDENTIFIER_AUTHORITY samba_unmapped = { 0, 0, 0, 0, 0, 22 }; GENERIC_MAPPING genMap; HANDLE hToken = NULL; DWORD desiredAccess = 0; @@ -1398,7 +1408,8 @@ NativeAccess( size = 0; (*tclWinProcs->getFileSecurityProc)(nativePath, OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION - | DACL_SECURITY_INFORMATION, 0, 0, &size); + | DACL_SECURITY_INFORMATION | LABEL_SECURITY_INFORMATION, + 0, 0, &size); /* * Should have failed with ERROR_INSUFFICIENT_BUFFER @@ -1431,7 +1442,8 @@ NativeAccess( if (!(*tclWinProcs->getFileSecurityProc)(nativePath, OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION - | DACL_SECURITY_INFORMATION, sdPtr, size, &size)) { + | DACL_SECURITY_INFORMATION | LABEL_SECURITY_INFORMATION, + sdPtr, size, &size)) { /* * Error getting owner SD */ @@ -1440,6 +1452,26 @@ NativeAccess( } /* + * As of Samba 3.0.23 (10-Jul-2006), unmapped users and groups are + * assigned to SID domains S-1-22-1 and S-1-22-2, where "22" is the + * top-level authority. If the file owner and group is unmapped then + * the ACL access check below will only test against world access, + * which is likely to be more restrictive than the actual access + * restrictions. Since the ACL tests are more likely wrong than + * right, skip them. Moreover, the unix owner access permissions are + * usually mapped to the Windows attributes, so if the user is the + * file owner then the attrib checks above are correct (as far as they + * go). + */ + + if(!GetSecurityDescriptorOwner(sdPtr,&pSid,&SidDefaulted) || + memcmp(GetSidIdentifierAuthority(pSid),&samba_unmapped, + sizeof(SID_IDENTIFIER_AUTHORITY))==0) { + HeapFree(GetProcessHeap(), 0, sdPtr); + return 0; /* Attrib tests say access allowed. */ + } + + /* * Perform security impersonation of the user and open the * resulting thread token. */ @@ -1515,16 +1547,6 @@ NativeAccess( Tcl_SetErrno(EACCES); return -1; } - /* - * For directories the above checks are ok. For files, though, - * we must still check the 'attr' value. - */ - if ((mode & W_OK) - && !(attr & FILE_ATTRIBUTE_DIRECTORY) - && (attr & FILE_ATTRIBUTE_READONLY)) { - Tcl_SetErrno(EACCES); - return -1; - } } return 0; } |