Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 775416 - sys-apps/sandbox: causes unlink() to return EACCES instead of ENOENT for missing pyc & pyo files
Summary: sys-apps/sandbox: causes unlink() to return EACCES instead of ENOENT for miss...
Status: CONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Sandbox Maintainers
URL:
Whiteboard:
Keywords:
Depends on: 256953
Blocks:
  Show dependency tree
 
Reported: 2021-03-11 09:36 UTC by Michał Górny
Modified: 2023-12-30 14:26 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-03-11 09:36:34 UTC
$ cat test.c 
#include <assert.h>
#include <stdio.h>
#include <unistd.h>

int main() {
	unlink("/usr/lib/python3.10/site-packages/locker.pyc");
	perror("wtf");
	return 0;
}
$ gcc test.c 
$ ./a.out 
wtf: No such file or directory


But inside sandbox:
$ ./a.out 
wtf: Permission denied


This breaks programs that verify unlink() errno value to atomically determine whether the file existed, particularly dev-lang/python's tests.
Comment 1 Sergei Trofimovich (RETIRED) gentoo-dev 2021-05-06 08:00:51 UTC
I'm surprised is does not flag sandbox violations as it's an attempt to mutate the filesystem outside the sandbox.

If file existed what what would prevent it from hitting sandbox violation?
Comment 2 Sergei Trofimovich (RETIRED) gentoo-dev 2021-05-06 08:12:12 UTC
SANDBOX_DEBUG=1 shows us that it happens because the path is already in sandbox predict handling mode. That explains why we don't see a sandbox violation:

$ ./a
 * absolute_path: /tmp/a
 * resolved_path: /tmp/a
 * ACCESS ALLOWED:  execve:       /tmp/a
 * absolute_path: /usr/lib/python3.10/site-packages/locker.pyc
 * resolved_path: /usr/lib/python3.10/site-packages/locker.pyc
 * ACCESS PREDICTED:  unlink:       /usr/lib/python3.10/site-packages/locker.pyc
wtf: Permission denied

It's set in https://gitweb.gentoo.org/proj/sandbox.git/tree/libsandbox/libsandbox.c#n855

		/* A very common bug (apparently) is for .py[co] files to fall out
		 * of sync with their .py source files.  Rather than trigger a hard
		 * failure, let's just whine about it.  Once python itself gets
		 * sorted out, we can drop this #256953.
		 */
		size_t len = strlen(resolv_path);
		if (len > 4) {
			const char *py = resolv_path + len - 4;
			if (!strcmp(py, ".pyc") || !strcmp(py, ".pyo")) {
				sbcontext->show_access_violation = false;
				goto out;
			}
		}
Comment 3 SpanKY gentoo-dev 2021-10-18 05:12:16 UTC
the underlying issue was claimed as fixed, but i don't think anyone validated it
Comment 4 Larry the Git Cow gentoo-dev 2021-10-22 04:20:07 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=60cff8d682fe7816ca0656d4da27e630855287e7

commit 60cff8d682fe7816ca0656d4da27e630855287e7
Author:     Mike Frysinger <vapier@gentoo.org>
AuthorDate: 2021-10-22 04:18:15 +0000
Commit:     Mike Frysinger <vapier@gentoo.org>
CommitDate: 2021-10-22 04:19:44 +0000

    libsandbox: drop old *.py[co] hack #775416
    
    With our eclasses & python frameworks responsible for generating
    these files now, we should be able to reject write attempts to these
    again.  Lets turn it back on and see what blows up.
    
    Bug: http://bugs.gentoo.org/256953
    Closes: https://bugs.gentoo.org/775416
    Signed-off-by: Mike Frysinger <vapier@gentoo.org>

 libsandbox/libsandbox.c | 14 --------------
 1 file changed, 14 deletions(-)
Comment 5 Larry the Git Cow gentoo-dev 2021-11-17 10:49:40 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=e9119fb9153d127d8a22a7301f522a5afd8a617d

commit e9119fb9153d127d8a22a7301f522a5afd8a617d
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2021-11-17 10:08:44 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2021-11-17 10:46:26 +0000

    dev-lang/python: use 'addpredict'
    
    The issue requiring the use of 'addwrite' seems to be fixed now.
    
    Bug: https://bugs.gentoo.org/775416
    Signed-off-by: Sam James <sam@gentoo.org>

 dev-lang/python/python-3.10.0_p1.ebuild     | 3 +--
 dev-lang/python/python-3.11.0_alpha1.ebuild | 3 +--
 dev-lang/python/python-3.11.0_alpha2.ebuild | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)
Comment 6 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2021-12-07 09:26:16 UTC
It's still happening the same way.
Comment 7 SpanKY gentoo-dev 2022-01-13 08:09:29 UTC
what exactly are you requesting here ?  that sandbox not throw an error if code attempts to unlink() paths it shouldn't ?  that goes against the point of having the sandbox.

so the test case you posted originally is WAI -- it triggers a violation and aborts the process.

unlinking files that don't exist and that don't trigger violations see ENOENT.

$ ./a.out
 * ACCESS DENIED:  unlink:        /usr/lib/python3.10/site-packages/locker.pyc
wtf: Permission denied
$ exit
Cleaning up sandbox process
============================= Gentoo path sandbox ==============================
The protected environment has been shut down.
 * ----------------------- SANDBOX ACCESS VIOLATION SUMMARY -----------------------
 * LOG FILE: "/var/log/sandbox/sandbox-32212.log"
 * 
VERSION 1.0
FORMAT: F - Function called
FORMAT: S - Access Status
FORMAT: P - Path as passed to function
FORMAT: A - Absolute Path (not canonical)
FORMAT: R - Canonical Path
FORMAT: C - Command Line

F: unlink
S: deny
P: /usr/lib/python3.10/site-packages/locker.pyc
A: /usr/lib/python3.10/site-packages/locker.pyc
R: /usr/lib/python3.10/site-packages/locker.pyc
C: ./a.out 
 * --------------------------------------------------------------------------------
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2023-12-30 14:26:28 UTC
I expect sandbox behavior to be consistent with Unix permission model.  When a regular user attempt to unlink a non-existing file inside a directory they don't have access to, unlink() fails with ENOENT, not EACCES.  I expect sandbox to do the same.

Or to put it simpler: I expect to have useful sandbox that I don't have to completely disable in order to be able to run Python's test suite.