KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
687 stars 228 forks source link

Malicious kOS scripts #778

Closed TDW89 closed 8 years ago

TDW89 commented 9 years ago

oooh this is fun i can log "stuff that is not a script" to "/log/koslog.txt". (log folder needs to exist first so can only be done on the archive). hadn't thought of doing that before.

Dunbaratu commented 9 years ago

Does that actually allow you to escape from the KSP directory and go to root? It's not supposed to allow that. If it does there's a bug.

TDW89 commented 9 years ago

um i think it only allows me to go down the file system not up will check though.

TDW89 commented 9 years ago

nope at least log "run for it" to "~/Desktop/escapee.ks". doesn't work.

NoPanShabuShabu commented 9 years ago

Yes you can.
log "This is a test" to "/../../../../NVIDIA/koslog4.txt" Handily made me a file in "C:/NVIDIA/". The only thing stopping me from writing elsewhere were file permissions.

TDW89 commented 9 years ago

Edit : content removed

Dunbaratu commented 9 years ago

Does anyone know if .NET has an equivalent to the UNIX chroot() ? That would be the proper secure way to deal with this. (the reason I care about the security is the potential for collaboration over telnet).

TDW89 commented 9 years ago

Edit: content removed

Dunbaratu commented 9 years ago

The simplest solution is to just deny all filenames containing any '\' or '/'. But I sort of wanted to allow the ability to descend DOWN into a subdirectory - just not up into a parent directory.

So how about:

Would that cover all the cases?

abenkovskii commented 9 years ago

In my view it's an awful idea to use string operations with file names unless you are implementing a module for path manipulation. If I were you I'd used something like this:

// pseudo code:
bool is_safe (file_name):
  path = file_name.normalize()
  result = false
  while path.has_parent() and not result:
    path = path.parent()
    result = path.same_path(scripts_folder)
  return result

This will solve the problem for all the file systems without leaving any special cases behind. P.S. I'm sure C# has it's own module for working with file names.

Dunbaratu commented 9 years ago

Hmm it should be possible to get the full-path equivalent name from an API call, and then walk it to see if it contains the intended 'chroot' or not. But I would worry about legitimate cases where someone has installed KSP under a symbolic link path, which they should be allowed to do, but asking the OS for the "real" name causes it to tell you about the hard path instead of the soft path, and therefore it always claims to be an illegal folder.

abenkovskii commented 9 years ago

What if you normalize both file_name and scripts_folder?

erendrake commented 9 years ago

We can get the a bunch of path definitions of any file before we write to it. Maybe at runtime we could find the absolute path of the script folder. Then when we are about to modify a file we check that that file has an absolute path that starts with the script path we found before.

I don't know if this would work for people who have symlinked in a subfolder. That being said I don't want us to spend all week on this for something that might not be needed. We do need to address the "virus" potential

abenkovskii commented 9 years ago

I think that a proper fix will be to implement a virtual file system with root under scripts folder in the real file system. It will also abstract out the differences in the path parsing in different OS.