This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #15763][BZ #14752] Restrict shm_open and shm_unlink to SHMDIR.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Florian Weimer <fweimer at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 23 Oct 2013 14:41:40 +0200
- Subject: Re: [PATCH][BZ #15763][BZ #14752] Restrict shm_open and shm_unlink to SHMDIR.
- Authentication-results: sourceware.org; auth=none
- References: <20131015073738 dot GA32465 at domone dot podge> <5264FB77 dot 7090400 at redhat dot com>
On Mon, Oct 21, 2013 at 12:01:27PM +0200, Florian Weimer wrote:
> On 10/15/2013 09:37 AM, OndÅej BÃlka wrote:
> >Hi, this patch adds validation restrict shm_open into a SHMDIR
> >directory.
> >
> >A manpage says that it is only recomanded for portability, it is ok to
> >break nonportable apps? If not then close 15763.
> >
> >As nobody bothered to test posix shmopen implementations I will not add
> >validation until its fixed.
> >
> >OK to commit?
>
> Looks okay to me in principle.
>
> >+ if (name[0] == '\0' || namelen > NAME_MAX || strchr (name, '/'))
>
> I think most of libc uses explicit checks: strchr (name, '/') != NULL.
>
> >+ if (name[0] == '\0' || namelen > NAME_MAX || strchr (name, '/'))
>
> Likewise.
>
> --
> Florian Weimer / Red Hat Product Security Team
--
OK now?
[BZ #14752], [BZ #15763]
* sysdeps/unix/sysv/linux/shm_open.c (shm_open, shm_unlink):
Validate name.
* rt/tst_shm.c: Add test for escaping directory.
diff --git a/rt/tst-shm.c b/rt/tst-shm.c
index f9d5ab0..cb4b1ee 100644
--- a/rt/tst-shm.c
+++ b/rt/tst-shm.c
@@ -134,6 +134,14 @@ do_test (void)
int status2;
struct stat64 st;
+ fd = shm_open ("/../escaped", O_RDWR | O_CREAT | O_TRUNC | O_EXCL, 0600);
+ if (fd != -1)
+ {
+ perror ("read file outside of SHMDIR directory");
+ return 1;
+ }
+
+
/* Create the shared memory object. */
fd = shm_open ("/shm-test", O_RDWR | O_CREAT | O_TRUNC | O_EXCL, 0600);
if (fd == -1)
diff --git a/sysdeps/unix/sysv/linux/shm_open.c b/sysdeps/unix/sysv/linux/shm_open.c
index 41d9315..482b49c 100644
--- a/sysdeps/unix/sysv/linux/shm_open.c
+++ b/sysdeps/unix/sysv/linux/shm_open.c
@@ -148,14 +148,15 @@ shm_open (const char *name, int oflag, mode_t mode)
while (name[0] == '/')
++name;
- if (name[0] == '\0')
+ namelen = strlen (name);
+
+ /* Validate the filename. */
+ if (name[0] == '\0' || namelen > NAME_MAX || strchr (name, '/') == NULL)
{
- /* The name "/" is not supported. */
__set_errno (EINVAL);
return -1;
}
- namelen = strlen (name);
fname = (char *) alloca (mountpoint.dirlen + namelen + 1);
__mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
name, namelen + 1);
@@ -237,14 +238,15 @@ shm_unlink (const char *name)
while (name[0] == '/')
++name;
- if (name[0] == '\0')
+ namelen = strlen (name);
+
+ /* Validate the filename. */
+ if (name[0] == '\0' || namelen > NAME_MAX || strchr (name, '/') == NULL)
{
- /* The name "/" is not supported. */
__set_errno (ENOENT);
return -1;
}
- namelen = strlen (name);
fname = (char *) alloca (mountpoint.dirlen + namelen + 1);
__mempcpy (__mempcpy (fname, mountpoint.dir, mountpoint.dirlen),
name, namelen + 1);