Skip to content

Commit 7499924

Browse files
committed
Be extra sure that no forbidden files are hanging around.
Hopefully this is not strictly required, but it is better to avoid a blank catch-all except clause when dealing with security details. The test may fail for example if the program manages to make the directory not writable by the user running CMS. In this case the removal fails, but CMS ignores it. I do not know if this could happen now, but in line of principle it may happen in the future if we change the owner model without remembering of this snippet.
1 parent 0e198e3 commit 7499924

1 file changed

Lines changed: 5 additions & 2 deletions

File tree

cms/grading/tasktypes/Batch.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# -*- coding: utf-8 -*-
33

44
# Contest Management System - http://cms-dev.github.io/
5-
# Copyright © 2010-2014 Giovanni Mascellani <mascellani@poisson.phc.unipi.it>
5+
# Copyright © 2010-2015 Giovanni Mascellani <mascellani@poisson.phc.unipi.it>
66
# Copyright © 2010-2014 Stefano Maggiolo <s.maggiolo@gmail.com>
77
# Copyright © 2010-2012 Matteo Boscariol <boscarim@hotmail.com>
88
# Copyright © 2012-2014 Luca Wehrstedt <luca.wehrstedt@gmail.com>
@@ -342,7 +342,10 @@ def evaluate(self, job, file_cacher):
342342
try:
343343
sandbox.remove_file(input_filename)
344344
except OSError as e:
345-
pass
345+
# Let us be extra sure that the file
346+
# was actually removed and we did not
347+
# mess up with permissions.
348+
assert not sandbox.file_exists(input_filename)
346349
sandbox.create_file_from_storage(
347350
input_filename,
348351
job.input)

0 commit comments

Comments
 (0)