Fix Time-of-check time-of-use filesystem race condition

[odaysec]

https://github.com/apache/trafficserver/blob/28710feefbcd5f10ce5def123f6fbd3e09fc1b79/src/tscore/ink_cap.cc#L407-L410

Fix the TOCTOU race condition, we should replace the chmod call with fchmod, which operates on a file descriptor rather than a file path. This ensures that the permissions are applied to the same file that was opened, regardless of any changes to the file system in the meantime. The fix involves:

1. Opening the file using open to obtain a file descriptor.
2. Using fchmod to change the permissions of the file referenced by the file descriptor.
3. Closing the file descriptor after the operation.

This approach ensures that the file being operated on is the same file that was opened, eliminating the race condition.

FIO01-C. Be careful using functions that use file names for identification

The following shows a case where a file is opened and then, if the opening was successful, its permissions are changed with chmod. However, an attacker might change the target of the file name between the initial opening and the permissions change, potentially changing the permissions of a different file.

char *file_name;
FILE *f_ptr;

/* Initialize file_name */

f_ptr = fopen(file_name, "w");
if (f_ptr == NULL)  {
  /* Handle error */
}

/* ... */

if (chmod(file_name, S_IRUSR) == -1) {
  /* Handle error */
}

fclose(f_ptr);

This can be avoided by using fchmod with the file descriptor that was received from opening the file. This ensures that the permissions change is applied to the very same file that was opened.

char *file_name;
int fd;

/* Initialize file_name */

fd = open(
  file_name,
  O_WRONLY | O_CREAT | O_EXCL,
  S_IRWXU
);
if (fd == -1) {
  /* Handle error */
}

/* ... */

if (fchmod(fd, S_IRUSR) == -1) {
  /* Handle error */
}

close(fd);

[JosiahWI]

@odaysec Doesn't the risk apply to the time window before the first chmod attempt?

I think there is a real concern that the file object we execute chmod on is not the one we decided we should chmod. Basically, the "time-of-check" is at the point where we determine that it is safe to chmod, not at the point where elevating_chmod is invoked. Doesn't this patch secure a file descriptor too late?

[JosiahWI]

I have looked at more of the code, and some additional context is useful: this is not a security issue in ATS right now; the elevating_chmod function is only called in one place, with the file descriptor already obtained. However, it seems reasonable to consider safer designs to ensure future changes are also secure.