summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordgp <dgp@users.sourceforge.net>2012-02-02 16:44:06 (GMT)
committerdgp <dgp@users.sourceforge.net>2012-02-02 16:44:06 (GMT)
commitbb750df4b42a35f9bc9ebd37914a8ac6c91d2a09 (patch)
treeea97947c31e62b6ec0f50ad8a3642734780afb8f
parent889858de26753cb512fda204a65783c308b7b576 (diff)
parent2669f7aa359f52c6ef4c3b7653581d63af47d62c (diff)
downloadtcl-bb750df4b42a35f9bc9ebd37914a8ac6c91d2a09.zip
tcl-bb750df4b42a35f9bc9ebd37914a8ac6c91d2a09.tar.gz
tcl-bb750df4b42a35f9bc9ebd37914a8ac6c91d2a09.tar.bz2
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.
-rw-r--r--ChangeLog10
-rw-r--r--win/tclWinFile.c70
2 files changed, 56 insertions, 24 deletions
diff --git a/ChangeLog b/ChangeLog
index 692b3a5..3edec76 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+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>
* tools/uniClass.tcl: [Frq 3473670]: Various Unicode-related
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;
}