Re: [AD] Possible bug in src/libc.c

[Stepan Roh <stepan@xxxxxxxxxx>]

On Fri, 16 Feb 2001, Olly Betts wrote:

> In message <Pine.LNX.4.30.0101281813270.957-100000@xxxxxxxxxx>, Stepan Roh wri
> tes:
> > if ((ff_info->stat.st_uid == geteuid()) {
> >     if ((ff_info->stat.st_mode & S_IWUSR) == 0))
> >          ffblk->ff_attrib |= FA_RDONLY;
> > } else if ((ff_info->stat.st_gid == getegid()) {
> >     if ((ff_info->stat.st_mode & S_IWGRP) == 0))
> >          ffblk->ff_attrib |= FA_RDONLY;
> > } else if ((ff_info->stat.st_mode & S_IWOTH) == 0) {
> >          ffblk->ff_attrib |= FA_RDONLY;
> > }
> >[...]
> >> It won't check against supplementary groups, which it
> >> should (to be perfect! :-) ).
>
> You might want to look at the access() function which you are almost trying
> to reinvent here

I know, I looked at it. Reasons why I choose this was: 1. it grew from old
one line code (which was wrong), 2. access() is much more time consuming
(but noone will ever see any difference, I think :-) ) - it does lookup in
dentry cache by call to stat() (on Linux of course :-) ), which we have
already in ff_info->stat. But if anyone wants to recommit it, he can
easily replace that by :

if (access (ffinfo->filename, W_OK) != 0)
	ffblk->ff_attrib |= FA_RDONLY;

>     The check is done with the process's real uid and gid, rather than with
>     the effective ids as is done when actually attempting an operation. This
>     is to allow set-UID programs to easily determine the invoking user's
>     authority.

There is also euidaccess() which tests for effective ids, but that's
glibc's extension. I checked its code and it is a little bit better, but
generally the same as my code (with exception to supplementary groups
checking). But if anyone is making Allegro programs setuid, the smallest
problem is that FA_RDONLY is not set on proper files. Making any program
setuid is a big security (and, in case of Allegro and svgalib or DGA,
system stability) hole. He will be punished by something else :-).

Have a nice day.

Stepan Roh