Fake handles (was: Re: New VMMaker/svn release)

David T. Lewis lewis at mail.msen.com
Wed Jan 4 12:22:43 UTC 2006


On Tue, Jan 03, 2006 at 10:41:29PM -0800, Andreas Raab wrote:
> Hi Dave -
> 
> > The fileID is an opaque value that just happens to correspond to some
> > data structure that we are not supposed to know about, but it is easier
> > to handle the issue in the Win32OSProcessPlugin that to try to design a 
> > platform independent solution that can be incorporated into FilePlugin for
> > all supported platforms. 
> 
> *Gasp* If I've ever seen "evil" code, this got to be it ;-) Not because 
> it's badly written but because it's designed to break the (supposed) 
> encapsulation of the file structure. If the file plugin code ever 
> changes (such as when removing the fileSize) that code will make the VM 
> explode.

Sure. In fact this has happened quite recently. The SQFile struct was
changed to handle some alignment issues for the 64 bit VM. I had to
put out an update to OSPP to accommodate the change. If nobody had
been maintaining OSPP, this would have caused a problem. Thankfully
the VM did not explode, although I did have some failing prims in the
unit tests.

Manipulating fileID is a Bad Thing. In the case of OSPP, the tradeoff
was between doing that particular bad thing versus attempting to
maintain a package of platform-specific extensions as part of the
FilePlugin, SocketPlugin, and so forth. I chose to implement the
extensions separately, and this has worked well for me.

> The main lesson for me here is that if it's *that* easy to fake a file 
> handle we've got a real problem with security. Basically, all of our I/O 
> operations are prone to this kind of attack and I don't even want to 
> think about all of the different ways in which this could be exploited.

Yes. I suspect this would be among the least of our security problems, but
you are absolutely right.

> In order to fix this, I just added a tiny little hash table to the 
> windows file plugin which checks whether a file handle is genuine, e.g., 
> has been produced inside the the file plugin or not. Since that hash 
> table is not publicly accessible you can't store into it, thus you can 
> no longer fabricate a random file handle.
> 
> BTW, this is not designed "just so your evil code breaks" - as far as I 
> am concerned it's actually a *major* step forward towards a secure VM 
> design because one of the effects this achieves is that the handle 
> (token) that's used in the plugin is actually a capability to invoke a 
> given set of primitives and no others, and it is solely at the plugins 
> discretion which set of primitives to assign to which handle. Nice, 
> simple, and secure (just as it should be).
> 
> One of the effects of that change is that it immediately makes the 
> session ID superfluous from a security POV. The worst that could ever 
> happen is that an "old" handle refers to a "new" file (if handles aren't 
> unique across sessions) but an invalid handle can never be used in any 
> of these operations. (note that from a security POV an old handle 
> referring to a new file is indistinguishable from cloning an existing 
> handle which is something that we don't deal with either right now - 
> fixing this would be straightforward if we were able to deal with oops 
> instead of third-party handles but that would require dealing with oop 
> relocation during GC; ouch).

This sounds like a pretty reasonable approach. It will have a rather
large impact on OSPP file operations (e.g. file locking on Unix), but
probably manageable. I would like to look it over and remind myself
of all the things that are going to break. If you can hold off until
say next week before diving into this I'd appreciate it.

> And now that we got this done ;-) we can start talking about how to 
> expose the functionality that you need in a nice and cross-platform way. 
> Like, accessing standard file handles: What's wrong with a 
> primitiveGetStdFileHandle that takes an integer argument for the kind of 
> standard handle to retrieve (0 - stdin, 1 - stdout, 2 - stderr)? Sounds 
> simple enough to me. Any others you'd need?

I don't really like the 0, 1, 2 convention, because it's an ancient
Unix convention, and really they are supposed to be treated as
opaque handles (seriously). It's probably OK to implement primitiveGetStdOut,
primitiveGetStdErr, and primitiveGetStdErr in the FilePlugin as long
as they are optional primitives and fail in reasonable ways.
that these are really still unix conventions that have been adopted
elsewhere, and may be rather platform-dependent. They behave differently
on Win32 and unix, and I don't know what RiscOS does about them. Tim?

Pipe handles are definitely needed, and given Squeak's current approach
to files and sockets, these should be treated as file handles. Anonymous
pipes don't have path names, so the process of opening them is of course
different.  Pipes are very platform-dependent, and I'm not sure this
belongs in the core plugins. I'll see if I can think of a way to do
this that is not too "evil" ;)

Dave




More information about the Vm-dev mailing list