Menu
▾
▴
#3125 file writable lies for certain XP directories
closed-fixed
9
2006-03-21
2005-05-01
No
Windows XP SP2 french, Tcl/Tk cvs HEAD
file writable "C:/Documents and
Settings/<account>/Mes Documents/mydir"
returns 1 if mydir does exist, or 0 if not.
This is correct.
file writable "C:/Documents and Settings/"
returns also 1, which is correct.
But:
file writable "C:/Documents and
Settings/<account>/Mes Documents"
returns 0 but it should be 1
<account> is my account name (with administrator
rights), this directory of course exists and I'm allowed to
write files in it.
Discussion
Logged In: YES
user_id=72656
While you can actually write to that directory, I think
Windows handles it specially. If I do through Explorer and
do Properties on my "My Documents" dir, it does clearly have
the "Read only" box checked, which is correctly reported by
Tcl as well.
Logged In: YES
user_id=1245417
All the directories I mention in my report show the read only
attribute if I look at their properties using Explorer,
even "mydir" which I created manually in "Mes Documents".
Logged In: YES
user_id=1245417
And I can't remove the read only attribute using Explorer. It
let me do it, but if I reopen the Properties box, it is here again.
Logged In: YES
user_id=1245417
And apparently this is by design of Windows. See
http://support.microsoft.com/kb/326549/en-us
Logged In: YES
user_id=72656
Which would imply that this is a Windows by-design item.
Logged In: YES
user_id=1245417
The fact that the read only box cannot be unchecked using
the Explorer is a Windows by-design item, yes.
But all the directories of my report have this attribute set.
Nevertheless, [file writable] returns 0 or 1 depending on which
directory it checks. Moreover, I *can* write files in all of them.
IMO this means that the way tcl implements the file writable
command should be revised to return 1 for all of them.
Otherwise, how can I check if a directory is writable or not?
In other words, there should be more/other checks than the
read only attribute (I know there is already more, see bug
749876, but this is apparently not enough).
Logged In: YES
user_id=1266631
Clearly from the MSDN document it states that the "Read-only
attribute" does not mean that the directory is not writable,
but instead means it is a "special customized folder". The
writeability is determined by the ACL list.
From MSDN : CAUSE
Unlike the Read-only attribute for a file, the Read-only
attribute for a folder is typically ignored by Windows,
Windows components and accessories, and other programs. For
example, you can delete, rename, and change a folder with
the Read-only attribute by using Windows Explorer. The
Read-only and System attributes is only used by Windows
Explorer to determine whether the folder is a special
folder, such as a system folder that has its view customized
by Windows (for example, My Documents, Favorites, Fonts,
Downloaded Program Files), or a folder that you customized
by using the Customize tab of the folder's Properties dialog
box.
Logged In: YES
user_id=1266631
Something like this would check the ACL list for writable
permission, written in C++ though.
bool is_writable(const char *path) {
char psd_buf[1024], *psd_ptr = psd_buf, *psd_alloc = 0;
DWORD sd_rlen;
if (!GetFileSecurity (path,
OWNER_SECURITY_INFORMATION
| GROUP_SECURITY_INFORMATION
| DACL_SECURITY_INFORMATION,
psd_buf, sizeof(psd_buf), &sd_rlen)) {
return false; // Failed ...
}
if (sizeof(psd_buf) < sd_rlen) {
psd_alloc = new char[sd_rlen];
DWORD psd_alloc_rlen;
if (!GetFileSecurity (path,
OWNER_SECURITY_INFORMATION
| GROUP_SECURITY_INFORMATION
| DACL_SECURITY_INFORMATION,
psd_alloc, sd_rlen,
&psd_alloc_rlen)
|| sd_rlen < psd_alloc_rlen) {
delete [] psd_alloc;
return false; // Failed ....
}
psd_ptr = psd_alloc;
sd_rlen = psd_alloc_rlen;
}
HANDLE hToken = NULL, hIToken = NULL;
if ( !OpenProcessToken ( GetCurrentProcess ( ),
TOKEN_DUPLICATE, &hToken ) ) {
if (psd_alloc)
delete [] psd_alloc;
return false;
}
BOOL status = DuplicateToken (hToken,
SecurityIdentification, &hIToken);
CloseHandle (hToken);
if (! status) {
if (psd_alloc)
delete [] psd_alloc;
return false;
}
GENERIC_MAPPING mapping = { FILE_GENERIC_READ,
FILE_GENERIC_WRITE,
FILE_GENERIC_EXECUTE,
FILE_ALL_ACCESS };
char pbuf[sizeof (PRIVILEGE_SET) + 3 * sizeof
(LUID_AND_ATTRIBUTES)];
DWORD plength = sizeof(pbuf), granted;
if (AccessCheck (psd_ptr, hIToken, FILE_WRITE_DATA,
&mapping,
(PPRIVILEGE_SET) pbuf, &plength,
&granted, &status))
retVal = true;
CloseHandle(hIToken);
if (psd_alloc)
delete [] psd_alloc;
return true;
}
Logged In: YES
user_id=32170
Can you suggest and test a patch to tclWinFile.c which
resolves this for you?
Logged In: YES
user_id=80530
dgp visitor is upset about [file writable]'s lies on windows.
[17:05] dgp 1193497, I think
[17:06] dgp he tracked it down to a fundamental difference
in meaning of permission bits on the two systems.
[17:07] dgp On unix, when a user lacks write permission to a
directory, the user cannot create/delete files in that
directory.
[17:07] dgp On Windows, when a users lacks write permission
to a directory, the user cannot delete the directory itself,
but may create/delete files within it as much as they like.
I would prefer to have [file writable] give
consistent information cross-platform, so
I can write portable scripts with it. Wipe
out OS Differences!
Logged In: YES
user_id=1245417
Hi all,
Anything new wrt this bug?
Is anybody planning to fix it?
I really need [file writable] to work correctly. My scripts are
currently not portable on Windows because of this bug.
Thanks.
Logged In: YES
user_id=72656
There is some demo code provided below. If someone wants to
turn that into a patch for the core, we can try it out.
Logged In: YES
user_id=1245417
I would be very thankful if somebody could derive a proper
patch from the code below.
I already had a deep look at this code, but I have to admit
that I'm not proficient enough to do it myself, sorry for that.
Logged In: YES
user_id=1266631
Below is a patch generated against the Tcl 8.4.11 source.
The specific file is win/tclWinFile.c rev 1.44.2.12 . The
patch implements and "NativeIsWritable" function. The
function needs to have the wide char implementation added.
The only call which needs to be updated is the
GetFileSecurity function which needs an entry in the
tclWinProcs defnition. The patch falls back to the old
implementation for wide char paths.
< if ((mode & W_OK) && (attr & FILE_ATTRIBUTE_READONLY)) {
---
> if ((mode & W_OK) && !NativeIsWritable(nativePath)) {
1365a1367,1442
> * NativeIsWritable --
> *
> * Determines if a path is writable. On windows this
> * requires more than a simple read only flag check for
directories
> * because windows uses this as a flag to customize
the folder
> * options. BUG 1193497
> *
> * Results:
> * 1 = writable, 0 = not.
> *
>
*----------------------------------------------------------------------
> */
> static int
> NativeIsWritable(nativePath)
> CONST TCHAR *nativePath;
> {
> char psd_buf[1024], *psd_ptr = psd_buf, *psd_alloc = 0;
> DWORD sd_rlen;
> HANDLE hToken = NULL;
> int retVal = 0;
>
> if (tclWinProcs->useWide) {
> /* not implemented -- fall back to old implementation */
> return
((*tclWinProcs->getFileAttributesProc)(nativePath) &
FILE_ATTRIBUTE_READONLY) ? 0 : 1;
> }
>
> if (!GetFileSecurity(nativePath,
> OWNER_SECURITY_INFORMATION
> | GROUP_SECURITY_INFORMATION
> | DACL_SECURITY_INFORMATION,
> psd_buf, sizeof(psd_buf), &sd_rlen)) {
> return 0; /* Failed ... */
> }
> if (sizeof(psd_buf) < sd_rlen) {
> DWORD psd_alloc_rlen;
> psd_alloc = (char *)ckalloc(sd_rlen);
> if (!GetFileSecurity(nativePath,
> OWNER_SECURITY_INFORMATION
> | GROUP_SECURITY_INFORMATION
> | DACL_SECURITY_INFORMATION,
> psd_alloc, sd_rlen, &psd_alloc_rlen)
> || sd_rlen < psd_alloc_rlen) {
> ckfree(psd_alloc);
> return 0; /* Failed .... */
> }
> psd_ptr = psd_alloc;
> sd_rlen = psd_alloc_rlen;
> }
> if ( OpenProcessToken ( GetCurrentProcess ( ),
TOKEN_DUPLICATE, &hToken ) ) {
> HANDLE hIToken = NULL;
> BOOL status = DuplicateToken (hToken,
SecurityIdentification, &hIToken);
> CloseHandle (hToken);
>
> if (status) {
> GENERIC_MAPPING mapping = { FILE_GENERIC_READ,
> FILE_GENERIC_WRITE,
> FILE_GENERIC_EXECUTE,
> FILE_ALL_ACCESS };
> char pbuf[sizeof (PRIVILEGE_SET) + 3 * sizeof
(LUID_AND_ATTRIBUTES)];
> DWORD plength = sizeof(pbuf), granted;
> retVal = AccessCheck (psd_ptr, hIToken,
FILE_WRITE_DATA, &mapping,
> (PPRIVILEGE_SET) pbuf,
&plength, &granted, &status) ? 1 : 0;
>
> CloseHandle(hIToken);
> }
> }
>
> if (psd_alloc)
> ckfree(psd_alloc);
>
> return retVal;
> }
> /*
>
*----------------------------------------------------------------------
> *
Logged In: YES
user_id=1266631
missing lines of the patch in the previous comment :
173a174
> static int NativeIsWritable(CONST TCHAR *path);
1336c1337
Logged In: YES
user_id=79902
Please find attached a patch file that implements the same
changes that Jason suggests, but recut as a UniDiff against
the HEAD (UniDiffs, created with the -u option to cvs diff,
are normally by far the easiest sorts of patches to apply
and review). The changes should apply equally well against
8.4, but I've not checked. I've got the formatting to follow
the core standards, and I've added a few comments to
indicate what *I* think is going on.
I do not know if the patch is correct (I *cannot* review it)
and I wonder whether using 1024 for the statuc buffer size
(as the original patch did) is a bit much. After all, stack
space is comparatively valuable.
unidiff vs. tclWinFile.c in HEAD
Logged In: YES
user_id=1266631
Below is a patch which implements the unicode call since
Tcl seems to use unicode for WinNT by default. It _corrects_
the functionality of the AccessCheck and removes the buffer
on the stack. The source is based on the Tcl 8.4.11 source
and encompasses the changes by dkf. The patch should still
be easily applied on the CVS HEAD.
The patch is generated with diff -u (3 files)
--- ../src/tcl8.4.11/win/tclWinInt.h 2004-05-03
11:01:36.000000000 -0700
+++ win/tclWinInt.h 2005-10-21 11:20:21.390410900 -0700
@@ -76,6 +76,8 @@
BOOL (WINAPI *getComputerNameProc)(WCHAR *,
LPDWORD);
DWORD (WINAPI *getCurrentDirectoryProc)(DWORD,
WCHAR *);
DWORD (WINAPI *getFileAttributesProc)(CONST TCHAR
*);
+ BOOL (WINAPI *getFileSecurityProc)(CONST TCHAR *,
SECURITY_INFORMATION,
+ PSECURITY_DESCRIPTOR, DWORD,
LPDWORD);
DWORD (WINAPI *getFullPathNameProc)(CONST
TCHAR *, DWORD nBufferLength,
WCHAR *, TCHAR **);
DWORD (WINAPI *getModuleFileNameProc)(HMODULE,
WCHAR *, int);
--- ../src/tcl8.4.11/win/tclWin32Dll.c 2005-06-06
14:04:46.000000000 -0700
+++ win/tclWin32Dll.c 2005-10-21
11:20:25.075710100 -0700
@@ -89,6 +89,8 @@
(BOOL (WINAPI *)(WCHAR *, LPDWORD))
GetComputerNameA,
(DWORD (WINAPI *)(DWORD, WCHAR *))
GetCurrentDirectoryA,
(DWORD (WINAPI *)(CONST TCHAR *))
GetFileAttributesA,
+ (BOOL (WINAPI *)(CONST TCHAR *,
SECURITY_INFORMATION,
+ PSECURITY_DESCRIPTOR, DWORD,
LPDWORD)) GetFileSecurityA,
(DWORD (WINAPI *)(CONST TCHAR *, DWORD
nBufferLength, WCHAR *,
TCHAR **)) GetFullPathNameA,
(DWORD (WINAPI *)(HMODULE, WCHAR *, int))
GetModuleFileNameA,
@@ -138,6 +140,8 @@
(BOOL (WINAPI *)(WCHAR *, LPDWORD))
GetComputerNameW,
(DWORD (WINAPI *)(DWORD, WCHAR *))
GetCurrentDirectoryW,
(DWORD (WINAPI *)(CONST TCHAR *))
GetFileAttributesW,
+ (BOOL (WINAPI *)(CONST TCHAR *,
SECURITY_INFORMATION,
+ PSECURITY_DESCRIPTOR, DWORD,
LPDWORD)) GetFileSecurityW,
(DWORD (WINAPI *)(CONST TCHAR *, DWORD
nBufferLength, WCHAR *,
TCHAR **)) GetFullPathNameW,
(DWORD (WINAPI *)(HMODULE, WCHAR *, int))
GetModuleFileNameW,
--- ../src/tcl8.4.11/win/tclWinFile.c 2005-06-22
14:23:32.000000000 -0700
+++ win/tclWinFile.c 2005-10-21 16:06:21.584662400 -0700
@@ -171,6 +171,7 @@
static int NativeStat(CONST TCHAR *path, Tcl_StatBuf
*statPtr, int checkLinks);
static unsigned short NativeStatMode(DWORD attr, int
checkLinks, int isExec);
static int NativeIsExec(CONST TCHAR *path);
+static int NativeIsWritable(CONST TCHAR *path);
static int NativeReadReparse(CONST TCHAR*
LinkDirectory,
REPARSE_DATA_BUFFER* buffer);
static int NativeWriteReparse(CONST TCHAR*
LinkDirectory,
@@ -1333,7 +1334,7 @@
return -1;
}
- if ((mode & W_OK) && (attr &
FILE_ATTRIBUTE_READONLY)) {
+ if ((mode & W_OK) && !NativeIsWritable(nativePath)) {
/*
* File is not writable.
*/
@@ -1360,6 +1361,102 @@
return 0;
}
+ /*
+ *----------------------------------------------------------------------
+ *
+ * NativeIsWritable --
+ *
+ * Determine if a path is writable. On Windows this
requires more than a
+ * simple read only flag check for directories
because Windows uses this
+ * as a flag to customize the folder options. [Bug
1193497]
+ *
+ * Results:
+ * 1 = writable, 0 = not.
+ *
+ * Side effects:
+ * May smash the last error value (errors are just
reported by this
+ * function as non-writable files).
+ *
+ *----------------------------------------------------------------------
+ */
+static int
+NativeIsWritable(
+ CONST TCHAR *nativePath)
+{
+ BYTE *secDescPtr = 0;
+ DWORD secDescLen = 0;
+ HANDLE hToken = NULL;
+ BOOL isWritable = FALSE;
+
+#define FILE_SEC_INFO_BITS \
+ OWNER_SECURITY_INFORMATION \
+ | GROUP_SECURITY_INFORMATION \
+ | DACL_SECURITY_INFORMATION
+
+ /*
+ * Read the security descriptor for the file or directory.
Note the
+ * first call obtains the size of the security descriptor.
+ */
+
+ if (!tclWinProcs->getFileSecurityProc(nativePath,
FILE_SEC_INFO_BITS,
+ NULL, 0, &secDescLen)) {
+ if (GetLastError() ==
ERROR_INSUFFICIENT_BUFFER) {
+ DWORD secDescLen2 = 0;
+ secDescPtr = (BYTE *) ckalloc(secDescLen);
+ if (!tclWinProcs->getFileSecurityProc
(nativePath, FILE_SEC_INFO_BITS,
+ secDescPtr, secDescLen, &secDescLen2) ||
(secDescLen < secDescLen2)) {
+ goto done;
+ }
+ } else {
+ goto done;
+ }
+ }
+
+ /*
+ * Get the security identity of the process and (if
successful) use it
+ * with the security descriptor of the file to find out if the
file really
+ * is writable.
+ */
+
+ if (OpenProcessToken(GetCurrentProcess(),
TOKEN_DUPLICATE, &hToken)) {
+ HANDLE hIToken = NULL;
+ BOOL status = DuplicateToken(hToken,
SecurityIdentification, &hIToken);
+
+ CloseHandle(hToken);
+ if (status) {
+ PRIVILEGE_SET privilegeSet;
+ DWORD privilegeSetLength = sizeof(privilegeSet);
+ DWORD granted; /* unused */
+ DWORD accessDesired = ACCESS_WRITE;
+
+ /* Initialize generic mapping structure to map all. */
+ GENERIC_MAPPING GenericMapping;
+ memset(&GenericMapping, 0xff, sizeof
(GENERIC_MAPPING));
+ GenericMapping.GenericRead = ACCESS_READ;
+ GenericMapping.GenericWrite = ACCESS_WRITE;
+ GenericMapping.GenericExecute = 0;
+ GenericMapping.GenericAll = ACCESS_READ |
ACCESS_WRITE;
+ MapGenericMask(&accessDesired,
&GenericMapping);
+
+ AccessCheck(secDescPtr, hIToken,
accessDesired,
+ &GenericMapping, &privilegeSet,
&privilegeSetLength,
+ &granted, &isWritable);
+ CloseHandle(hIToken);
+ }
+ }
+
+ /*
+ * Release any temporary space we may be using and
convert the writability
+ * value into a boolean for return.
+ */
+
+ done:
+ if (secDescPtr) {
+ ckfree(secDescPtr);
+ }
+ return isWritable != 0;
+}
+
/*
*----------------------------------------------------------------------
*