cms-dev / cms

Contest Management System
http://cms-dev.github.io/
GNU Affero General Public License v3.0
898 stars 362 forks source link

Sandbox does not restrict file access and execution #376

Open kapf opened 9 years ago

kapf commented 9 years ago

The sandbox does not restrict writing to local files and allows access to a large part of the directory tree, in particular everything in /usr/. Here are some examples how this can be exploited:

stefano-maggiolo commented 9 years ago

Tentative solution to the first part (and #309): https://github.com/stefano-maggiolo/cms/commits/master (last 3 commits). I'd like to add your patch as well. My addition still lacks the whitelisting on compilation (haven't checked, but I'm pretty sure you can write to a file with C's preprocessor).

Let me know what you think, thanks!

stefano-maggiolo commented 9 years ago

I submitted the whitelisting of writable files. I'm not sure what could be a proper solution for the other two problems.

artikz commented 9 years ago

Is there any reason to allow executing and reading of writable files (0777)? If it would be changed to writing only (0722), it will solve part 2 of the second problem (creating and executing a file). I tried this with a simple C++ program and it worked.

As for the first part of the second problem (executing files from /usr), here are some possible solutions:

kapf commented 9 years ago

Thanks for the fix! A few comments:

The access should be either write-only (as artikz said) or read-and-execute-only. Assuming you consider compilation unsafe, you need to be careful with symlinks when overwriting permissions (since the symlink may point outside the sandbox). I'm not aware of a language where you can create files during compilation, though.

Change open(path, "w") to open(path, "w").close() so that you don't depend on a refcounting Python interpreter.

Some languages (C, C++, Pascal) can be statically linked, so there's no need for /usr or any other system files to exist during runtime. Together with the more restrictive permissions, this solves problem 2 at least for those languages.

Other languages (Python, Java) may be restricted by white- and blacklisting files by hand. The execl-Trick works with Python too, so one might want prevent executing /usr/bin/perl. Having no restriction by default should be fine, as long as you can manage such a whitelist in a config file.

stefano-maggiolo commented 9 years ago
  1. I will change permission from 0777 to 0722 - don't know why I didn't think of it...
  2. I will add close() as well (not that I don't trust Python to close the file, but it is more readable).
  3. I skimmed through the CPP docs and it seems I for once overestimated its power, it shouldn't be possible to mess with external files during compilation through it. This is good because preliminary tests in closing down compilations were unsuccessful (too many temp files).
  4. I will investigate on a good way to make the import of /usr dependent on the language to solve 2 partially. Sadly I don't think that whitelisting files would be an easy fix as isolate doesn't allow such a fine level of control.
artikz commented 9 years ago

Not sure whether it will help, but seems that remounting /usr with noexec flag and making hard link to /usr/bin/python2.7 inside evaluation sandbox does what we want. Not checked with java though.

gollux commented 6 years ago

As for the first problem: I do not think it is a good idea to re-use the sandbox used in executing the solution for running the checker. Generally, it is not safe to recycle sandboxes without cleaning them up first. There might be symlinks, hardlinks, crazy permissions, dragons etc. in it.

Second, if you do not like Python to be executable inside the sandbox, just do not put it there. This is much easier than inventing complex access control schemes. It might be nice to let CMS configure the parameters of the sandbox (like what should be mounted where) in more pleasant way, though.

gollux commented 2 years ago

For analysis for these and other attacks, please see my paper from last year's IOI conference: https://mj.ucw.cz/papers/secgrad.pdf

kenneth-kho commented 6 days ago

I noticed the following sentence by OP: _The sandbox does not restrict writing to local files and allows access to a large part of the directory tree, in particular everything in /usr/. Access to all header files in /usr/include. Is it allowed to #include <boost/graph/dijkstra_shortest_paths.hpp>?_

I am curious, what if the host wishes to allow #include <boost/graph/dijkstra_shortest_paths.hpp> as easy as #include <iostream>, what should the host do?