Re: Small ZipFile patch

2003-08-14 Thread Tom Tromey
 Brian == Brian Jones [EMAIL PROTECTED] writes:

 Sounds like we need this in ClassLoader and in any other class that
 performs security checks in its constructor.

Brian This is still a TODO item, yes?

I fixed this in ClassLoader.  I haven't looked to see whether any
other classes have a security check in their constructor.  (Well, I
know that Window does, but I think it doesn't need this treatment,
since it won't throw a SecurityException.)

Tom


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


Re: Small ZipFile patch

2003-08-12 Thread Brian Jones
Tom Tromey [EMAIL PROTECTED] writes:

  Jeroen == Jeroen Frijters [EMAIL PROTECTED] writes:
 
 Jeroen Sun's ClassLoader has a hack that prevents this from being exploitable:
 Jeroen http://www.securingjava.com/chapter-five/chapter-five-8.html
 
 Sounds like we need this in ClassLoader and in any other class that
 performs security checks in its constructor.

This is still a TODO item, yes?

Brian
-- 
Brian Jones [EMAIL PROTECTED]


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


Re: Small ZipFile patch

2003-03-11 Thread Stephen Crawley

[EMAIL PROTECTED] said:
 kaffe 1.0.7 doesn't throw any exceptions but also doesn't write to the
 file.

 Kissme CVS+Classpath CVS correctly throws SecurityException then dumps
 core.

 gij from CVS gives the interesting:

 Exception in thread main java.lang.ExceptionInInitializerError ***
 Got java.lang.NoClassDefFoundError: gnu.gcj.runtime.NameFinder while
 trying to print stack trace. Aborted

 Blackdown-1.4.1-beta correctly gives AccessControlException and then:

 Unexpected Signal : 11 occurred at PC=0x403A264C Function=(null)+0x403A
 264C Library=/opt/j2sdk1.4.1/jre/lib/i386/client/libjvm.so 

For the record, the testcase gives a JVM crash with JDK 1.4.0-b92 on Linux.

-- Steve




___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-05 Thread Jeroen Frijters
Tom Tromey wrote:
 Jeroen Another interesting trick with the finalizer is creating
 Jeroen instances of classes that have a private constructor! The
 Jeroen attached runtime.j creates an instance of (a subclass of)
 Jeroen java.lang.Runtime.
 
 Interesting test case.
 
 With gij this prints `null', but that's probably because the GC and
 finalization don't actually occur.
 
 Jeroen It could be considered a bug in Sun's verifier that it allows
 Jeroen a class without a constructor, what do the other VMs do with
 Jeroen this code?
 
 Both Sun 1.4 and IBM 1.3 print a non-null `runtime' object.
 
 Have you read this?
 
 http://www.lsd-pl.net/documents/javasecurity-1.0.0.pdf

Not sure. I have the pdf sitting on my desktop, so either I did or I'm
planning to ;-)

 It seems like your technique could be also used to circumvent the
 security check in the ClassLoader constructor.
 
 
 I wonder what Sun has to say about this.

Sun's ClassLoader has a hack that prevents this from being exploitable:
http://www.securingjava.com/chapter-five/chapter-five-8.html

Regards,
Jeroen


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


Re: Small ZipFile patch

2003-03-05 Thread Tom Tromey
 Jeroen == Jeroen Frijters [EMAIL PROTECTED] writes:

Jeroen Sun's ClassLoader has a hack that prevents this from being exploitable:
Jeroen http://www.securingjava.com/chapter-five/chapter-five-8.html

Sounds like we need this in ClassLoader and in any other class that
performs security checks in its constructor.

Tom


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-04 Thread Jeroen Frijters

Aaron M. Renn wrote:
 Jeroen Frijters ([EMAIL PROTECTED]) wrote:
  BTW, the design of the 1.0 I/O classes is less than 
 stellar. Since the
  FileDescriptor class is the owner of the native resource, 
 it should have
  had a finalizer, instead of FileInputStream and 
 FileOutputStream (for
  some reason RandomAccessFile doesn't have a finalizer). I 
 guess I should
  be a bit mild in my criticism, finalizers weren't very well 
 understood
  back then...
 
 I am in the process of correcting this.

My gripe really wasn't directed at the Classpath implementation, but at
the Sun implementation. How compatible do we want to be in this case?
Compatible and broken or a little less compatible and not so broken ;-)

Regards,
Jeroen


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-04 Thread Jeroen Frijters

Aaron M. Renn wrote:
 Jeroen Frijters ([EMAIL PROTECTED]) wrote:
   Jeroen Frijters ([EMAIL PROTECTED]) wrote:
BTW, the design of the 1.0 I/O classes is less than 
   stellar. Since the
FileDescriptor class is the owner of the native resource, 
   it should have
had a finalizer, instead of FileInputStream and 
   FileOutputStream (for
some reason RandomAccessFile doesn't have a finalizer). I 
   guess I should
be a bit mild in my criticism, finalizers weren't very well 
   understood
back then...
   
   I am in the process of correcting this.
  
  My gripe really wasn't directed at the Classpath 
 implementation, but at
  the Sun implementation. How compatible do we want to be in 
 this case?
  Compatible and broken or a little less compatible and not 
 so broken ;-)
 
 I think we want to do the right thing.  I believe it was you yourself
 who pointed out a condition whereby a FileInputStream closing the file
 descriptor in finalization could potentially lead to 
 problems.  Particularly
 since you can use the getFD() call to return a valid file descriptor
 that would suddenly get un-valid if your FileInputStream happened to
 be garbage collected out from under you.  Better to let FileDescriptor
 handle its own finalization.  Actually, its much better if 
 the application
 manually closes the stream (which would close the underlying 
 file descriptor)
 because there is no guarantee that anything will ever get garbage
 collected.

Putting the finalize in FileDescriptor should be safe (once
FileDescriptor is an opaque object). That way you can be sure that a
FileDescriptor will only be closed by finalization if it isn't being
used anymore, even in the case of sharing. The only downside to this
would be that code developed under Classpath might behave erratically
when run under a Sun VM. I remember when I first started using Java
(beginning of 1997) I spent a couple of days tracking down a weird bug
that turned out to be caused by my (shared) FileDescriptor being closed
when I didn't expect it (because a temporary FileInputStream was being
finalized at some random point in time).

Regards,
Jeroen


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


Re: Small ZipFile patch

2003-03-04 Thread Tom Tromey
 Jeroen == Jeroen Frijters [EMAIL PROTECTED] writes:

Jeroen BTW, the design of the 1.0 I/O classes is less than
Jeroen stellar. Since the FileDescriptor class is the owner of the
Jeroen native resource, it should have had a finalizer, instead of
Jeroen FileInputStream and FileOutputStream (for some reason
Jeroen RandomAccessFile doesn't have a finalizer).

It isn't a compatibility problem to decide what class should have a
nontrivial finalizer.  We're free to rearrange things here.

In libgcj we have a finalizer in FileDescriptor.  I think this is the
only place you should have a finalizer that closes the underlying
native file descriptor.  Otherwise you run into problems here and
there; we had some bug reports about this a couple years ago.

Tom


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


Re: Small ZipFile patch

2003-03-04 Thread Mark Wielaard
On Tue, 2003-03-04 at 15:11, Jeroen Frijters wrote:
 Hi,
 
 ZipFile has a finalizer (it shouldn't!) and that tries to close the
 RandomAccessFile, even the RandomAccessFile never was created, this
 causes an NPE during finalization, not a big deal, but since .NET 1.1
 decided to print these to the console, it's kind of distracting.

Ewh. That can only happen when the constructor threw an exception. Hmmm,
interesting fact that the finalizer is also called in such cases. Hadn't
realized that. It means that when finalize is called the Object could be
in some very unexpected state.

If it really helps you, we can install your patch. But it seems you have
a bit of a problem here in your VM. Finalizer are allowed to throw
exceptions and this should not cause such side effects as printing to
some stream. What happens in other cases in your VM? Wouldn't it make
more sense to encapsulate any java finalize() method (if an class
overrides the default one in Object) with something that catches any
exceptions so that the .NET runtime doesn't print them to some output
channel?

 ZipFile really shouldn't have a finalizer though (if you have a pure
 Java implementation). Only classes that own native resources should
 have finalizers.

Actually, it should have a finalizer since the spec says it has a
finalizer. And according to the spec that finalizer will call the
close() method of that object.It is easy to follow the spec here, I
don't think we shouldn't do it if it is so easy.  It helps people to
migrate from some proprietary implementation to our free one. 

 BTW 2, in gnu\java\nio\ServerSocketChannelImpl.java and
 gnu\java\nio\SocketChannelImpl.java both have method called
 finalize*r* and that probably should be finalize (although since these
 classes do not seem to own any native resources, they really don't
 need a finalizer).

Michael, could you look at this?

Cheers,

Mark



___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-04 Thread Jeroen Frijters
Mark Wielaard wrote:
 On Tue, 2003-03-04 at 15:11, Jeroen Frijters wrote:
  Hi,
  
  ZipFile has a finalizer (it shouldn't!) and that tries to close the
  RandomAccessFile, even the RandomAccessFile never was created, this
  causes an NPE during finalization, not a big deal, but 
 since .NET 1.1
  decided to print these to the console, it's kind of distracting.
 
 Ewh. That can only happen when the constructor threw an 
 exception. Hmmm,
 interesting fact that the finalizer is also called in such 
 cases. Hadn't
 realized that. It means that when finalize is called the 
 Object could be in some very unexpected state.

Yep. You can do some very tricky things with this. For every non-final
class with a non-final finalize it is possible to obtain an initialized
reference to an instance of that class *without* running a constructor
by taking advantage of the fact that the finalizer runs even if the
constructor was never invoked.

 If it really helps you, we can install your patch. But it 
 seems you have
 a bit of a problem here in your VM. Finalizer are allowed to throw
 exceptions and this should not cause such side effects as printing to
 some stream. What happens in other cases in your VM? Wouldn't it make
 more sense to encapsulate any java finalize() method (if an class
 overrides the default one in Object) with something that catches any
 exceptions so that the .NET runtime doesn't print them to some output
 channel?

Yes, you're right, ignore the patch, I'll fix my VM.

  ZipFile really shouldn't have a finalizer though (if you have a pure
  Java implementation). Only classes that own native resources should
  have finalizers.
 
 Actually, it should have a finalizer since the spec says it has a
 finalizer. And according to the spec that finalizer will call the
 close() method of that object.It is easy to follow the spec here, I
 don't think we shouldn't do it if it is so easy.  It helps people to
 migrate from some proprietary implementation to our free one. 

In that case the spec is broken. I really don't have a problem with it
being there, but I just want to point out that it is really bad design
to do anything in a finalizer other than releasing native resources. If
ZipFile is implemented using RandomAccessFile which uses FileDescriptor
which has a finalize, the FileDescriptor finalize may run before the
ZipFile finalize and thus the close will fail (unpredictably). This is
why it is bad design.

Regards,
Jeroen


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-04 Thread Mark Wielaard
Hi,

On Tue, 2003-03-04 at 23:16, Jeroen Frijters wrote:
 Yep. You can do some very tricky things with this. For every non-final
 class with a non-final finalize it is possible to obtain an initialized
 reference to an instance of that class *without* running a constructor
 by taking advantage of the fact that the finalizer runs even if the
 constructor was never invoked.

Ugh. That is terrible for trying to keep some sane security framework.
I tried the attached class which overrides RandomAccessFile. It first
installs a SecurityManager to prevent the class from actually writing to
some file, but then tries anyway by using the back-from-dead object
returned by the finalizer. The results are interesting:

kaffe 1.0.7 doesn't throw any exceptions but also doesn't write to the
file.

Kissme CVS+Classpath CVS correctly throws SecurityException then dumps
core.

gij from CVS gives the interesting:

Exception in thread main java.lang.ExceptionInInitializerError
*** Got java.lang.NoClassDefFoundError: gnu.gcj.runtime.NameFinder while
trying
to print stack trace.
Aborted

Blackdown-1.4.1-beta correctly gives AccessControlException and then:

Unexpected Signal : 11 occurred at PC=0x403A264C
Function=(null)+0x403A264C
Library=/opt/j2sdk1.4.1/jre/lib/i386/client/libjvm.so

Eeew.

Good night,

Mark
import java.io.*;

public class IRAF extends RandomAccessFile
{
static RandomAccessFile raf;

IRAF(String file) throws IOException
{
  super(file, rw);
}

protected void finalize()
{
  raf = this;
}

public static void main(String args[])
{
  System.setSecurityManager(new SecurityManager());
  try { new IRAF(args[0]); } catch (Throwable t) { t.printStackTrace(); }

  while (raf == null)
  {
	new Object(); // Generate some garbage till the finalizer triggers.
  }

  try { raf.write(0xff); } catch (Throwable t) { t.printStackTrace(); }
}
}
___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-04 Thread Mark Wielaard
Hi,

Forgot the most important one for sure:

$ mono --noinline --share-code bin/ikvm.exe IRAF testfile
java.lang.SecurityException: Operation not allowed
at java.lang.SecurityManager.checkPermission(unknown)
at java.lang.SecurityManager.checkWrite(unknown)
at java.io.RandomAccessFile.init(unknown)
at java.io.RandomAccessFile.init(unknown)
at IRAF.main(unknown)
at IRAF.main(unknown)
Killed

Note that it is killed because it rapidly eats up all memory in the
system.

Cheers,

Mark



___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


RE: Small ZipFile patch

2003-03-04 Thread Jeroen Frijters
Mono uses a conservative GC, so may be they just don't notice that IRAF
is garbage. On .NET ikvm runs fine:
C:\ikvm IRAF foo
java.lang.SecurityException: Operation not allowed
at java.lang.SecurityManager.checkPermission(Unknown Source)
at java.lang.SecurityManager.checkWrite(Unknown Source)
at java.io.RandomAccessFile.init(Unknown Source)
at java.io.RandomAccessFile.init(Unknown Source)
at IRAF.main(Unknown Source)
at IRAF.main(Unknown Source)
java.lang.NullPointerException
at java.io.RandomAccessFile.write(Unknown Source)
at IRAF.main(Unknown Source)
at IRAF.main(Unknown Source)

Another interesting trick with the finalizer is creating instances of
classes that have a private constructor! The attached runtime.j creates
an instance of (a subclass of) java.lang.Runtime. Doesn't work on ikvm,
because it ignores new without a corresponding call to the
constructor. It could be considered a bug in Sun's verifier that it
allows a class without a constructor, what do the other VMs do with this
code?

Regards,
Jeroen


  
  
 -Original Message-
 From: Mark Wielaard [mailto:[EMAIL PROTECTED] 
 Sent: Wednesday, March 05, 2003 00:07
 To: Jeroen Frijters
 Cc: [EMAIL PROTECTED]
 
 Hi,
 
 Forgot the most important one for sure:
 
 $ mono --noinline --share-code bin/ikvm.exe IRAF testfile
 java.lang.SecurityException: Operation not allowed
 at java.lang.SecurityManager.checkPermission(unknown)
 at java.lang.SecurityManager.checkWrite(unknown)
 at java.io.RandomAccessFile.init(unknown)
 at java.io.RandomAccessFile.init(unknown)
 at IRAF.main(unknown)
 at IRAF.main(unknown)
 Killed
 
 Note that it is killed because it rapidly eats up all memory in the
 system.
 
 Cheers,
 
 Mark
 
 


runtime.j
Description: runtime.j


runtime.class
Description: runtime.class
___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath


Re: Small ZipFile patch

2003-03-04 Thread Tom Tromey
 Jeroen == Jeroen Frijters [EMAIL PROTECTED] writes:

Jeroen Another interesting trick with the finalizer is creating
Jeroen instances of classes that have a private constructor! The
Jeroen attached runtime.j creates an instance of (a subclass of)
Jeroen java.lang.Runtime.

Interesting test case.

With gij this prints `null', but that's probably because the GC and
finalization don't actually occur.

Jeroen It could be considered a bug in Sun's verifier that it allows
Jeroen a class without a constructor, what do the other VMs do with
Jeroen this code?

Both Sun 1.4 and IBM 1.3 print a non-null `runtime' object.

Have you read this?

http://www.lsd-pl.net/documents/javasecurity-1.0.0.pdf

It seems like your technique could be also used to circumvent the
security check in the ClassLoader constructor.


I wonder what Sun has to say about this.

Tom


___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath