Ex41/devpkg: is access deceiving us? Should we use it?

While doing exercise 41 (devpkg), during review I found the man page for access to explain that it answers a different question than we could assume (without reading the manpage).
The text from the manpage of access() reads as follows:

  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.   Similarly,  for
   the  root  user, the check uses the set of permitted capabilities rather than the set of effec‐
   tive capabilities; and for non-root users, the check uses an empty set of capabilities.

This set me thinking if we aren’t using the wrong function here. One paragraph below it says:

   This allows set-user-ID programs and capability-endowed programs to easily determine the invok‐
   ing  user's  authority.  In other words, access() does not answer the "can I read/write/execute
   this file?" question.  It answers a  slightly  different  question:  "(assuming  I'm  a  setuid
   binary)  can  the  user  who invoked me read/write/execute this file?", which gives set-user-ID
   programs the possibility to prevent malicious users from causing them to read files which users
   shouldn't be able to read.

That would imply that if a directory is root only RWX, a call to access() would return an error while the directory is still there, just not readable for our read UID / GID.

In the notes of the manpage it discourages use to access() to check access before opening:

      Warning: Using these calls to check if a user is authorized to, for example, open a file before
      actually doing so using open(2) creates a security hole, because the  user  might  exploit  the
      short  time  interval between checking and opening the file to manipulate it.  For this reason,
      the use of this system call should be avoided.  (In the example just described, a safer  alter‐
      native  would  be to temporarily switch the process's effective user ID to the real ID and then
      call open(2).)

With respect to exercise 41 in devpkg within the program db.c we have a function DB_init() where we use the following code:

 95 int DB_init()                                                                                     
 96 {                                                                                                    
 97     apr_pool_t *p = NULL;
 98     apr_pool_initialize();
 99     apr_pool_create(&p, NULL);
101     if (access(DB_DIR, W_OK | X_OK) == -1)
102     {                  
103         apr_status_t rc = apr_dir_make_recursive(DB_DIR, APR_UREAD | APR_UWRITE | APR_UEXECUTE |
104                           APR_GREAD | APR_GWRITE | APR_GEXECUTE, p);
105         check(rc == APR_SUCCESS, "Failed to make database dir: %s.", DB_DIR);
107     }
109     if (access(DB_FILE, W_OK) == -1)
110     {                         
111         FILE *db = DB_open(DB_FILE, "w");
112         check(db, "Can not open database: %s.", DB_FILE);
113         DB_close(db);
114     }                                                                                           
116     apr_pool_destroy(p);                                                     
117     return 0;
119 error:
120     apr_pool_destroy(p);            
121     return 1;
122 }

As you can see, the code uses access to check before creating a directory which will barf if the real UID is not permitted to access the directory while the effective userid is allowed to access the directory. The creation will fail.

In the second use of access, the check will not fail afaics because the open with 'w' mode will smoothly open the file.
The function DB_open uses fopen which will truncate the file with mode w. That hurts. Especially if you just added a lot of information.

So the question is: should we alter the use of access? Or should we not use access at all? But what to use in stead of access.

Has this ever bitten you in real life programs? Or is the manpage being overly protective?

Kind regards, Guus.

That’s really interesting. Yes, weird little edge cases like this bite people all the time, probably because you’re told “use access to test if you can get to a file/dir” and then “oh except in this, this,this oh actually use this other one we created last year.”

If you want, make the change, and then shoot me a PR on the github project and I’ll pull it in. Maybe the APR library has an even better function to use too?