-
Notifications
You must be signed in to change notification settings - Fork 360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sandbox does not set file size limit #309
Comments
It is a very bad idea to publish details on security issues when there are no ready solutions. You should have communicated them privately. At the moment, I don't have time to check how difficult it is to implement file system quota in |
As pointed out by Stefano, there is a |
I haven't been able to make it work at the moment, but I'm confident it
On Wed Apr 30 2014 at 8:47:50 AM, Giovanni Mascellani <
|
Another solution could be to set RLIMIT_NOFILE to an appropriate value (5, letting contestants open two files beside standard input/output/error) and RLIMIT_FSIZE to the same value as RLIMIT_AS. |
Here http://pastebin.com/7kTW9bs4 is a patch that implements the fix suggested in the last comment. |
I don't think your proposed fix is advisable, as some languages (probably Java, Python, etc.) need the interpreter to access files on disk to load parts of the standard library. Therefore a global constant limit on the number of open file descriptors isn't feasible. |
True, that would be a problem... |
I made some experiments with the quota option, and the result is that it seems to work - you have to enable quota in the kernel, enable options usrquota,grpquota for the filesystem holding /tmp, and then run quotacheck -vguma as root to create the quota files. The problem is that tmpfs does not support quotas, so that will break running cms on a system that mounts /tmp on a tmpfs. Yet another method to do the check could be enabling RLIMIT_FSIZE and then providing isolate with a whitelist of paths that may be opened for writing: using inotify, we find out if a file different from the ones in the whitelist is opened for writing and in that case we kill the user process. |
I've tried these steps with Ubuntu 13.10 Server. Ubuntu doesn't mount tmpfs Also I'm not sure whether it is expected or not, but seems that quota is On Wed, Apr 30, 2014 at 5:27 PM, veluca93 notifications@github.com wrote:
Artem Iglikov |
Well, that's a problem as far as I know: if for whatever reason a sandbox does not get deleted (for example, the worker crashes/raises an exception) it could fill up the user quota. The simplest solution so far would be to deny write permissions on a sandbox folder, then create empty files with write permissions inside it (eg. output.txt) |
http://pastebin.com/jui88R3r |
Copied to cms-dev/isolate#6 (but not closing, since it also involves how CMS uses isolate). |
Whitelist of files is implemented and submitted (at least during evaluation). |
One nice solution would be to mount a dedicated The problem is that this could be too memory expensive in some cases: if there are big input and output files, at some point they have to be in RAM twice; one copy in the process running the solution and one copy in the Another solution would be to create, format and loop-mount a dedicated image for each job. This would have the same advantages of the All in all, it seems to be a reasonable alternative for me and I could try to investigate. Let me know what do you think. |
@veluca93 Why in your patch you swap the calls to |
Provides a partial and temporary fix for cms-dev#309.
Provides a partial and temporary fix for cms-dev#309.
I have no idea - possibly I had just swapped around some lines around there and in the end i left those in a different order... |
Restricting file size does not really help. You can create as many files as you wish. Even if you try to use RLIMIT_NOFILE to cap the number of available file descriptors, you can still close one file and open another whenever you wish. The only correct solution I see is to use proper filesystem quotas instead, which (as already mentioned) is supported by isolate. A tmpfs on /tmp/ should not be an issue any longer since recent version of isolate do not use /tmp/ anyway for security reasons. I see no way how to evade the memory limit using mmap. Without cgroups, isolate limit the total size of virtual memory available to the process, which includes mmap'ed files. In cgroup mode, it limits the total memory available to the cgroup, which also includes cached pages of files (yes, you can mmap a file larger than the available memory, but then the contents of the file will not be present in the memory at once, so it will be terribly slow). So the only remaining task is to teach CMS how to ask isolate for a quota. |
Perfect, seems like a good solution to me. |
The sandbox does not set a limit on the size of the files created by the solution. This can lead to various problems.
takes only two seconds to complete on my computer and creates a 10GB file.
The text was updated successfully, but these errors were encountered: