[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
FYI: java.io.File security check fixes
From: |
Mark Wielaard |
Subject: |
FYI: java.io.File security check fixes |
Date: |
Sun, 18 Apr 2004 16:35:31 +0200 |
Hi,
This fixes a couple of File.security failures in Mauve.
It makes sure that we do the needed security check first before trying
to do any actual file operations. Unneeded checks have been eliminated.
2004-04-18 Mark Wielaard <address@hidden>
* java/io/File.java (canWrite): Only do checkWrite() security
check, use internal methods for actual actions.
(createTempFile): Don't do security checks for temp files that
won't be created.
(setReadOnly): Do checkWrite() security check before trying to
do anything else.
(renameTo): Add checkWrite() security check for destination file.
Only one related mauve failure remains with this:
java.lang.SecurityException: Unexpected check: (java.io.FilePermission
/tmp/mauve-testdir/test-dir-write1082297238707.tmp write)
FAIL: gnu.testlet.java.io.File.security: dir.canWrite - unexpected exception
(number 1)
This is because we actually use createTempFile() to check that a
directory is writable and that generates some extra security checks. It
would be nice to just check whether or not a directory is writable
directly without trying to do a couple of explicit file creation calls.
But currently our native layer doesn't offer such an option.
Cheers,
Mark
Index: java/io/File.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/File.java,v
retrieving revision 1.40
diff -u -r1.40 File.java
--- java/io/File.java 17 Apr 2004 17:29:18 -0000 1.40
+++ java/io/File.java 18 Apr 2004 14:26:54 -0000
@@ -173,22 +173,21 @@
*/
public boolean canWrite()
{
- // We still need to do a SecurityCheck since exists() only checks
- // for read access
+ // First do a SecurityCheck before doing anything else.
checkWrite();
// Test for existence. This is required by the spec
- if (!exists())
+ if (!existsInternal(path))
return false;
- if (!isDirectory())
+ if (!isDirectoryInternal(path))
return canWriteInternal(path);
else
try
{
/* If the separator is '\' a DOS-style-filesystem is assumed
and a short name is used, otherwise use a long name.
- WARNGIN: some implementation of DOS-style-filesystems also
+ WARNING: some implementation of DOS-style-filesystems also
accept '/' as separator. In that case the following code
will fail.
*/
@@ -961,10 +960,10 @@
throw new IOException("Cannot determine system temporary
directory");
directory = new File(dirname);
- if (!directory.exists())
+ if (!directory.existsInternal(directory.path))
throw new IOException("System temporary directory "
+ directory.getName() + " does not exist.");
- if (!directory.isDirectory())
+ if (!directory.isDirectoryInternal(directory.path))
throw new IOException("System temporary directory "
+ directory.getName()
+ " is not really a directory.");
@@ -994,7 +993,7 @@
String filename = prefix + System.currentTimeMillis() + suffix;
file = new File(directory, filename);
}
- while (file.exists());
+ while (file.existsInternal(file.path));
}
else
{
@@ -1011,7 +1010,7 @@
String filename = prefix + java.lang.Integer.toHexString(n) +
suffix;
file = new File(directory, filename);
}
- while (file.exists());
+ while (file.existsInternal(file.path));
}
// Verify that we are allowed to create this file
@@ -1020,6 +1019,7 @@
sm.checkWrite(file.getAbsolutePath());
// Now create the file and return our file object
+ // XXX - FIXME race condition.
createInternal(file.getAbsolutePath());
return file;
}
@@ -1045,13 +1045,13 @@
*/
public boolean setReadOnly()
{
+ // Do a security check before trying to do anything else.
+ checkWrite();
+
// Test for existence.
- if (!exists())
+ if (!existsInternal(path))
return false;
- // We still need to do a SecurityCheck since exists() only checks
- // for read access
- checkWrite();
return setReadOnlyInternal(path);
}
@@ -1192,6 +1192,7 @@
public synchronized boolean renameTo(File dest)
{
checkWrite();
+ dest.checkWrite();
// Call our native rename method
return renameToInternal(path, dest.path);
}
signature.asc
Description: This is a digitally signed message part
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- FYI: java.io.File security check fixes,
Mark Wielaard <=