summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordgp <dgp@users.sourceforge.net>2012-02-02 17:35:59 (GMT)
committerdgp <dgp@users.sourceforge.net>2012-02-02 17:35:59 (GMT)
commit59bcf7fa71bb5fe35152ec943c954a95797105fc (patch)
treeea10d68cdcf600089963f79042478233d48b22c7
parent3ffba781139cb0ecc4ccf4011ae453ba247011a6 (diff)
parentc55e690196895fde865b1093ba33f375f8c98dba (diff)
downloadtcl-59bcf7fa71bb5fe35152ec943c954a95797105fc.zip
tcl-59bcf7fa71bb5fe35152ec943c954a95797105fc.tar.gz
tcl-59bcf7fa71bb5fe35152ec943c954a95797105fc.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.
-rw-r--r--ChangeLog9
-rw-r--r--win/tclWinFile.c72
2 files changed, 56 insertions, 25 deletions
diff --git a/ChangeLog b/ChangeLog
index f4a6b72..d065f45 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+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.
+
2012-02-01 Donal K. Fellows <dkf@users.sf.net>
* doc/AddErrInfo.3: [Bug 3482614]: Documentation nit.
diff --git a/win/tclWinFile.c b/win/tclWinFile.c
index 08e5b42..78128d6 100644
--- a/win/tclWinFile.c
+++ b/win/tclWinFile.c
@@ -1540,20 +1540,30 @@ NativeAccess(
}
}
-#ifndef UNICODE
+ if (mode == F_OK) {
+ /*
+ * File exists, nothing else to check.
+ */
+
+ return 0;
+ }
+
if ((mode & W_OK)
- && (attr & FILE_ATTRIBUTE_READONLY)) {
+ && (attr & FILE_ATTRIBUTE_READONLY)
+ && !(attr & FILE_ATTRIBUTE_DIRECTORY)) {
/*
- * We don't have the advanced 'GetFileSecurity', and our
- * attributes say the file is not writable. If we do have
- * 'GetFileSecurity', 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);
return -1;
}
-#endif /* !UNICODE */
if (mode & X_OK) {
if (!(attr & FILE_ATTRIBUTE_DIRECTORY) && !NativeIsExec(nativePath)) {
@@ -1572,15 +1582,15 @@ 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) {
+#ifndef UNICODE
+ {
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;
@@ -1596,7 +1606,8 @@ NativeAccess(
size = 0;
GetFileSecurity(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
@@ -1629,7 +1640,8 @@ NativeAccess(
if (!GetFileSecurity(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
*/
@@ -1638,6 +1650,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.
*/
@@ -1714,18 +1746,8 @@ 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;
- }
}
+#endif /* !UNICODE */
return 0;
}