suid-root doesn't have effect

Question

A program from apue.

#include "apue.h"
#include <fcntl.h>

int main(int argc, char *argv[])
{
    if(argc!=2)
        err_quit("usage: a.out <pathname>");

    if(access(argv[1], R_OK)<0)
        err_ret("access error for %s",argv[1]);
    else
        printf("read access OK\n");
    if (open(argv[1], O_RDONLY)) {
        err_ret("open error for %s", argv[1]);
    } else {
        printf("open for reading OK\n");
    }

    return 0;
}

I compiled it to an executable named 4-2 and I changed the owner and set the suid this is the output of ls -l:

-rwsr-xr-x 1 root sinners 8490 Jan  7 18:50 4-2*

and /etc/shadow:

-rw------- 1 root root 421 Jan  4 01:29 /etc/shadow

But when I run it:

user% ./4-2 /etc/shadow 
access error for /etc/shadow: Permission denied
open error for /etc/shadow: Permission denied

Can someone explain why I am getting these access and open errors?

Answer 1

Quoting access() manpage:

The check is done using the calling process's real UID and GID, rather than the effective IDs as is done when actually attempting an operation (e.g., open(2)) on the file. This allows set-user-ID programs to easily determine the invoking user's authority.

Set-user-ID bit makes the process's effective UID equal to the file owner, but the real user ID remains unchanged (i.e. remains what it was before exec()).

You can use setreuid() to modify both effective UID and real UID of the process or you can use open() to determine privileges. I recommend the second solution especially since you already call open() anyway. You can check whether errno is equal to EPERM to find out whether insufficient permissions was the reason open() failed.

The call to open() fails in your code, because you use the return value from open() as the if condition. I guess the call actually succeeds and returns you a valid, non-zero (zero being already taken by stdin) file descriptor making the control go into the error-handling branch. The error-handling branch of the if displays EPERM error message since this is the last error which occurred due to access() call. The condition to enter the error handling branch should check whether open() returned -1.