From 9b9a88cd34149fc95d3ffdd23d657e7b3daf0c7d Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 11 Jan 2012 21:24:55 +0000 Subject: Contributed patch aiming to fix [file writable] problems on Windows & Samba. --- win/tclWinFile.c | 65 +++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/win/tclWinFile.c b/win/tclWinFile.c index 54cf0cf..64b2a0e 100644 --- a/win/tclWinFile.c +++ b/win/tclWinFile.c @@ -1550,14 +1550,25 @@ NativeAccess( return -1; } + if (mode == F_OK) { + /* + * File exists, nothing else to check. + */ + + return 0; + } + if ((mode & W_OK) - && (tclWinProcs->getFileSecurityProc == NULL) - && (attr & FILE_ATTRIBUTE_READONLY)) { + && (attr & FILE_ATTRIBUTE_READONLY) + && !(attr & FILE_ATTRIBUTE_DIRECTORY)) { /* - * 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. + * 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); @@ -1587,9 +1598,12 @@ NativeAccess( * 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, grantedAccess = 0; @@ -1605,7 +1619,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 @@ -1638,7 +1653,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 */ @@ -1647,6 +1663,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. */ @@ -1723,17 +1759,6 @@ NativeAccess( 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; } -- cgit v0.12