filelock icon indicating copy to clipboard operation
filelock copied to clipboard

File permission of lock file

Open ghost opened this issue 3 years ago • 6 comments

The creation of a lockfile with …

lock = FileLock("/var/lock/foo.lock")

… leads to these file permissions: -rwxr-xr-x

Is there any way to prevent that the lock file becomes an executable with root ownership?

(Version: 3.0.12-2 in Ubuntu 20.04)

ghost avatar Oct 25 '21 14:10 ghost

Can't you just change the file permissions after the lock file is created?

gaborbernat avatar Oct 25 '21 22:10 gaborbernat

Sure. But this is a potential security problem and should not happen at all. Creating files which are world wide executable is quite the opposite to the principle of least surprise.

ghost avatar Oct 26 '21 05:10 ghost

Feel free to open a PR with your proposed solution.

gaborbernat avatar Oct 26 '21 05:10 gaborbernat

Ok. I just have a question about the code:

mode = (
            os.O_WRONLY  # open for writing only
            | os.O_CREAT
            | os.O_EXCL  # together with above raise EEXIST if the file specified by filename exists
            | os.O_TRUNC  # truncate the file to zero byte
        )
        try:
            fd = os.open(self._lock_file, mode)

If I understand the documentation of os.open and the C runtime docs correctly these parameters should be called flags instead of mode and mode would then instead define the file permissions like S_IRUSR, or am I missing something?

ghost avatar Oct 26 '21 20:10 ghost

Your reading of the documentation is correct. The value of mode in the filelock code is passed into os.open's flags argument. This doesn't matter because os.open never sees the name of the variable being passed in, but I agree that mode should be renamed to flags to be less confusing. And then the third argument (= mode keyword argument) to os.open could be passed in whichever notation is preferred (os.S_IRUSR | os.S_IWUSR or 0o600, for example). I'm not sure what platform-specific tweaks might be necessary, especially on the Windows implementation.

JustAnotherArchivist avatar Jan 04 '22 20:01 JustAnotherArchivist

Sure. But this is a potential security problem and should not happen at all. Creating files which are world wide executable is quite the opposite to the principle of least surprise.

The lock file is just an empty file ? Why is it a security problem ? I understand your motivation, this file should not be executable. It would be nice if you have an explication, because you seem to know the potential security problems.

axoroll7 avatar Aug 21 '22 08:08 axoroll7

@gaborbernat - I believe the recent PRs should resolve this one. Please let me know if you disagree or if you believe any additional work is needed here.

jahrules avatar Mar 24 '23 12:03 jahrules