On Wed, May 05, 2004 at 12:28:34PM +1000, John Newbigin wrote:Thanks for the feedback. I have modified the procedure based on your suggestions. Obviously it can be modified as required.
Here is a more complete check procedure.
Yes, a check of all path components is preferable.
I have used this code so I hope it is secure :)
Unfortunately, no.
John.
int verify_socket_path(char *name)
{
int i = 0;
char x;
int result = 0;
struct stat buf; name = strdup(name);
if(!name)
{
return -1;
} while(name[i])
{
if(name[i] == '/')
{
x = name[i + 1];
name[i + 1] = 0;
//printf("path = %s\n", name);
stat(name, &buf);
//printf("uid = %d gid = %d\n", buf.st_uid, buf.st_gid);
if(buf.st_uid != 0)
{
result = -1;
fprintf(stderr, "directory %s not owned by root\n", name);
}
if(buf.st_mode & S_IWGRP)
{
result = -1;
fprintf(stderr, "group write access to %s\n", name);
}
if(buf.st_mode & S_IWOTH)
{
result = -1;
fprintf(stderr, "other write access to %s\n", name);
}
name[i + 1] = x;
}
i++;
} free(name);
return result;
}
if(buf.st_uid != 0)
{
// make sure there is no user write access
if(buf.st_mode & S_IWUSR)
{
result = -1;
fprintf(stderr, "non root user has write access to %s\n", name);
}
}
If a non-root user owns a directory, the user should be assumed to have write access to it. You must not check for S_IWUSR, that is largely irrelevant. This is because the user can chmod a directory he owns after your check has run.
if(buf.st_gid != 0)
{
// make sure there is no group write access
if(buf.st_mode & S_IWGRP)
{
result = -1;
fprintf(stderr, "non root group has write access to %s\n", name);
}
}
And this check is buggy in "the opposite" way: you must not check for GID 0 because it is not special to the kernel in any way and generally there's no valid reason to consider it trusted.
// make sure there is no group write access
if(buf.st_mode & S_IWOTH)
{
result = -1;
fprintf(stderr, "all users have write access to %s\n", name);
}
This one is OK, but I suggest that you combine it with the S_IWGRP check above to simplify the code. I don't see much need to have different error messages for the three cases.
-- John Newbigin - Computer Systems Officer School of Information Technology Swinburne University of Technology Melbourne, Australia http://www.it.swin.edu.au/staff/jnewbigin
_______________________________________________ Pam-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/pam-list
