Hi,

David Fries writes:

Currently the shared memory region that gimp uses to communicate to and from plugins is readable and writable by every user on the system. This is not good. I don't know what data or control information gimp puts in this shared region, but someone could at least view or corrupt your working image. Anyway it isn't too hard to fix.

As far as I know plugins can only be run as the same user id as the gimp. Unless this isn't the case the following patch needs to be applied.

Index: plug-in/plug-in.c =================================================================== RCS file: /cvs/gnome/gimp/app/plug-in/plug-in.c,v retrieving revision 1.180
diff -u -r1.180 plug-in.c
--- plug-in/plug-in.c 2002/05/21 10:58:30 1.180 +++ plug-in/plug-in.c 2002/06/12 02:16:58 @@ -192,7 +192,7 @@
*/

#ifdef HAVE_SHM_H
- shm_ID = shmget (IPC_PRIVATE, TILE_WIDTH * TILE_HEIGHT * 4, IPC_CREAT | 0777); + shm_ID = shmget (IPC_PRIVATE, TILE_WIDTH * TILE_HEIGHT * 4, IPC_CREAT | 0600);
if (shm_ID == -1)
g_message ("shmget() failed: Disabling shared memory tile transport.");

looks reasonable. Could you please open a bug-report on http://bugzilla.gnome.org/. Thanks.

Salut, Sven

David Fries writes:

Currently the shared memory region that gimp uses to communicate to and from plugins is readable and writable by every user on the system. This is not good. I don't know what data or control information gimp puts in this shared region, but someone could at least view or corrupt your working image. Anyway it isn't too hard to fix.

As far as I know plugins can only be run as the same user id as the gimp. Unless this isn't the case the following patch needs to be applied.

Yes, the plug-ins are simply forked and thus have the same uid. The patch looks like the right thing to do to me. If nobody objects for some reason, it will be applied to both the stable and unstable trees.

thanks for spotting this, --mitch

David Fries writes:

Currently the shared memory region that gimp uses to communicate to and from plugins is readable and writable by every user on the system. This is not good. I don't know what data or control information gimp puts in this shared region, but someone could at least view or corrupt your working image. Anyway it isn't too hard to fix.

As far as I know plugins can only be run as the same user id as the gimp. Unless this isn't the case the following patch needs to be applied.

Yes, the plug-ins are simply forked and thus have the same uid. The patch looks like the right thing to do to me. If nobody objects for some reason, it will be applied to both the stable and unstable trees.

thanks for spotting this, --mitch

might as well patch the configure script (which has the 0777) so that the test during configure is the same as during the application runtime ;-)

On 12 Jun 2002, Michael Natterer wrote:

David Fries writes:

Yes, the plug-ins are simply forked and thus have the same uid. The patch looks like the right thing to do to me. If nobody objects for some reason, it will be applied to both the stable and unstable trees.

thanks for spotting this, --mitch

You should put a big notice that there is a security fix in the NEWS file for the versions released. Perhaps an annoncement to BugTrax would be in order, as well.

I wonder if anyone in the dark ages is still using gimp 1.0 -- I'm sure it has the same flaw.

Rockwalrus

Hi,

David Fries writes:

Currently the shared memory region that gimp uses to communicate to and from plugins is readable and writable by every user on the system. This is not good. I don't know what data or control information gimp puts in this shared region, but someone could at least view or corrupt your working image. Anyway it isn't too hard to fix.

As far as I know plugins can only be run as the same user id as the gimp. Unless this isn't the case the following patch needs to be applied.

Index: plug-in/plug-in.c =================================================================== RCS file: /cvs/gnome/gimp/app/plug-in/plug-in.c,v retrieving revision 1.180
diff -u -r1.180 plug-in.c
--- plug-in/plug-in.c 2002/05/21 10:58:30 1.180 +++ plug-in/plug-in.c 2002/06/12 02:16:58 @@ -192,7 +192,7 @@
*/

#ifdef HAVE_SHM_H
- shm_ID = shmget (IPC_PRIVATE, TILE_WIDTH * TILE_HEIGHT * 4, IPC_CREAT | 0777); + shm_ID = shmget (IPC_PRIVATE, TILE_WIDTH * TILE_HEIGHT * 4, IPC_CREAT | 0600);
if (shm_ID == -1)
g_message ("shmget() failed: Disabling shared memory tile transport.");

On Wed, 12 Jun 2002 13:20:45 -0700 (PDT), "Nathan Carl Summers" wrote:

On 12 Jun 2002, Michael Natterer wrote:

Yes, the plug-ins are simply forked and thus have the same uid. The patch looks like the right thing to do to me. If nobody objects for some reason, it will be applied to both the stable and unstable trees.

You should put a big notice that there is a security fix in the NEWS file for the versions released. Perhaps an annoncement to BugTrax would be in order, as well.

I don't think that the problem is so serious. It can only be exploited locally and AFAIK it does not open any significant security holes because the shared memory area is only used for exchanging image tiles between the plug-ins and the core. So the only thing that could be done by a local attacker is to insert some nasty stuff in the image that is being processed by a plug-in, assuming that they win the race between the core and the plug-in. The bug should be fixed, but the window of opportunity for malicious uses of this shared memory segment seems to be rather small so it does not deserve any big announcement.

Unfortunately, I think that fixing this bug may introduce some new problems: some plug-ins may run under a different user id than the main program. For example, xscanimage may be installed with a setuid bit on some systems if this is required in order to access the scanner. I don't know how the real and effective user id are used in this case, but this may prevent the plug-in from running correctly.

Also, I think that some old systems (AIX? HP-UX?) had problems with shared memory segments unless they were created with the mode 777. This is very vague and I cannot find any information about that, so maybe this is just a brain fart on my part.

In any case, I don't think that we should be too fast for releasing this patch because it may cause more problems than it solves. We really need more testing and feedback from users of various UN*X systems, especially those who have to run some plug-ins setuid in order to access some special devices or files.

-Raphaël

On Thu, Jun 13, 2002 at 05:48:58PM +0200, Raphaël Quinet wrote:

I don't think that the problem is so serious. It can only be exploited locally and AFAIK it does not open any significant security holes because the shared memory area is only used for exchanging image tiles between the plug-ins and the core. So the only thing that could be done by a local attacker is to insert some nasty stuff in the image that is being processed by a plug-in, assuming that they win the race between the core and the plug-in. The bug should be fixed, but the window of opportunity for malicious uses of this shared memory segment seems to be rather small so it does not deserve any big announcement.

It is a security issue anyways. It could be exploited in fun ways. Say, there are plugins written in interpreted languages. If they assume that the shared memory is safe, they might be tricked into interpreting code (after all, we've already seen buffer overflows in jpeg, so this is not at all theoretical).

Unfortunately, I think that fixing this bug may introduce some new problems: some plug-ins may run under a different user id than the main program. For example, xscanimage may be installed with a setuid bit on some systems if this is required in order to access the scanner. I don't know how the real and effective user id are used in this case, but this may prevent the plug-in from running correctly.

Fix the problems, then. If xscanimage is setuid, then it can switch its effective id to be able to access the segment. Let me remind you that, with unix semantics, the rights are only checked at open, and are then attached to the file descriptor, even if you later lose your privileges.

Also, I think that some old systems (AIX? HP-UX?) had problems with shared memory segments unless they were created with the mode 777. This is very vague and I cannot find any information about that, so maybe this is just a brain fart on my part.

This is quite possible, but it is no excuse to keep a security hole around. In the worst case, write a configure test, and resort to mode 777 only if nothing else works.

Again, no reason to expose sane systems to security holes elsewhere.

In any case, I don't think that we should be too fast for releasing this patch because it may cause more problems than it solves. We really need more testing and feedback from users of various UN*X systems, especially those who have to run some plug-ins setuid in order to access some special devices or files.

If you start dealing with such problems in this way, you'll never get things solved. The only way to ensure plugin issues get fixed.

In any case, if a plugin needs to be setuid, then it had better be written by somewhat security-conscious people (or you've got a whole larger set of problems...), so fixing a shared memory ownership issue on that end is going to be a breeze.

Hi,

RaphaXl Quinet writes:

Unfortunately, I think that fixing this bug may introduce some new problems: some plug-ins may run under a different user id than the main program. For example, xscanimage may be installed with a setuid bit on some systems if this is required in order to access the scanner. I don't know how the real and effective user id are used in this case, but this may prevent the plug-in from running correctly.

Also, I think that some old systems (AIX? HP-UX?) had problems with shared memory segments unless they were created with the mode 777. This is very vague and I cannot find any information about that, so maybe this is just a brain fart on my part.

In any case, I don't think that we should be too fast for releasing this patch because it may cause more problems than it solves. We really need more testing and feedback from users of various UN*X systems, especially those who have to run some plug-ins setuid in order to access some special devices or files.

I think we should do a gimp-1.2.4-pre1 release. It wouldn't hurt to get some testing on the other changes that we've made to the gimp-1-2 branch. If we do a pre-release, we could as well apply some more patches that have been hold back because they need more testing.

Salut, Sven

I am shocked this is not being considered a security problem.

Hi,

Theo de Raadt writes:

I am shocked this is not being considered a security problem.

calm down, it is. It's already fixed in CVS and we will do a release shortly after we've verified that it doesn't break things.

Salut, Sven

Theo de Raadt writes:

I am shocked this is not being considered a security problem.

calm down, it is. It's already fixed in CVS and we will do a release shortly after we've verified that it doesn't break things.

Well, the attitude of uninformed denial still sucks...

On Thu, 13 Jun 2002, Marc Espie wrote:

On Thu, Jun 13, 2002 at 05:48:58PM +0200, Raphaël Quinet wrote:

Also, I think that some old systems (AIX? HP-UX?) had problems with shared memory segments unless they were created with the mode 777. This is very vague and I cannot find any information about that, so maybe this is just a brain fart on my part.

This is quite possible, but it is no excuse to keep a security hole around. In the worst case, write a configure test, and resort to mode 777 only if nothing else works.

It should default to no shared memory if the proper permissions don't work. (There could, of course, be a sufficiently omninous-sounding option to configure that would use 777 if the correct permissions don't work; I suggest --enable-shm-security-hole)

In any case, if a plugin needs to be setuid, then it had better be written by somewhat security-conscious people (or you've got a whole larger set of problems...), so fixing a shared memory ownership issue on that end is going to be a breeze.

Never assume that just because someone makes something setuid they know what they are doing. (also don't assume it's always setuid root).

Rockwalrus

On Thu, Jun 13, 2002 at 06:08:17PM -0700, Nathan Carl Summers wrote:

In any case, if a plugin needs to be setuid, then it had better be written by somewhat security-conscious people (or you've got a whole larger set of problems...), so fixing a shared memory ownership issue on that end is going to be a breeze.

Never assume that just because someone makes something setuid they know what they are doing. (also don't assume it's always setuid root).

Maybe I should just post directly from my @openbsd.org address. My comment was somewhat tongue-in-cheek.

OF COURSE, quite a few people write Unix programs not knowing what they're doing. And sadly, quite a few people write setuid stuff not knowing what they are doing, at all. If this shm issue comes from some programmer `needing it' for some setuid plugin of his, it is very obvious (and sad) that programmer doesn't know enough about Unix programming to be writing setuid programs...

On Thu, 13 Jun 2002 14:58:34 -0600, "Theo de Raadt" wrote:

Theo de Raadt writes:

I am shocked this is not being considered a security problem.

calm down, it is. It's already fixed in CVS and we will do a release shortly after we've verified that it doesn't break things.

Well, the attitude of uninformed denial still sucks...

I suppose that you are refering to my previous message. Note that I did not deny the fact that it is a bug that must be fixed. I just wanted to mention that the fix (which is already in CVS anyway) could break things for some people and we should do some testing before releasing the patch in a hurry. In other words, we should not consider this patch in isolation because we may have to modify some other parts of the GIMP if we want to avoid breaking it for some operating systems or for some specific configurations. Maybe there is nothing to change, in which case the patch could be released immediately. But maybe there is, so we should at least do some testing (and I am doing that right now for several versions of Linux and Solaris).

Also, I was specifically replying to Rockwalrus' suggestion that we should have a "big notice" about this security fix and maybe publish it on Bugtraq. I thought that it was a bit excessive, that's why I wrote: "The bug should be fixed, but the window of opportunity for malicious uses of this shared memory segment seems to be rather small so it does not deserve any big announcement." I do not consider this to be an "attitude of uninformed denial." If this is how it was perceived, then I am sorry for that. Maybe I should have used a better wording. I am a quite security-conscious person and I certainly do not want to leave any security hole open.

-Raphaël